StanfordAHA / CGRAFlow

Integration test for entire CGRA flow
BSD 3-Clause "New" or "Revised" License
12 stars 2 forks source link

Fresh sixteen pr #58

Closed cdonovick closed 6 years ago

cdonovick commented 6 years ago

see https://travis-ci.org/StanfordAHA/CGRAFlow/builds/362858868

cdonovick commented 6 years ago

This is https://github.com/StanfordAHA/CGRAFlow/tree/fresh_sixteen with every branch pointed at master

leonardt commented 6 years ago

The following changes are missing in the Makefile diff between fresh_sixteen and master versus sixteen and master.

@@ -352,7 +329,7 @@ build/%_CGRA_out.raw: build/%_pnr_bitstream

    @cd $(VERILATOR_TOP);    \
    build=../../../build;   \
-   ./run.csh top_tb.cpp -hackmem           \
+   ./run.csh top_tb.cpp          \
        $(QVSWITCH)              \
        $(MEM_SWITCH)                       \
        -config $${build}/$*_pnr_bitstream  \
@@ -312,10 +288,11 @@ endif
         # the bitstream) versus a separately-decoded version of the bitstream,
         # to make  sure they match
    @echo; echo Checking $*_annotated against separately-decoded $*_annotated...
-   @echo "% bsa_verify $*_pnr_bitstream $*_annotated"
-   @CGRAGenerator/testdir/bsa_verify.csh $(QVSWITCH) \
-       build/$*_annotated \
-       -cgra $(filter %.txt, $?)
+   @echo "% bsa_verify.csh" $(QVSWITCH) build/$*_annotated -cgra $(filter %.txt, $?)
+   @echo Temporarily NOT doing the bsa_verify check
+#  @CGRAGenerator/testdir/bsa_verify.csh $(QVSWITCH) \
+#      build/$*_annotated \
+#      -cgra $(filter %.txt, $?)
@@ -235,24 +221,12 @@ else
      $(filter %.txt, $?) 2>&1 | head -n 40 | tee -a test/compare_summary.txt
 endif

-build/cgra_info_4x4.txt:
-   @echo; echo Making $@ because of $?
-   @echo "CGRA generate (generates 4x4 CGRA + connection matrix for pnr)"
-   cd CGRAGenerator; ./bin/generate.csh $(QVSWITCH) || exit 13
-   cp CGRAGenerator/hardware/generator_z/top/cgra_info.txt build/cgra_info_4x4.txt
-
-build/cgra_info_8x8.txt:
-   @echo; echo Making $@ because of $?
-   @echo "CGRA generate (generates 8x8 CGRA + connection matrix for pnr)"
-   cd CGRAGenerator; export CGRA_GEN_USE_MEM=1; ./bin/generate.csh $(QVSWITCH) || exit 13
-   cp CGRAGenerator/hardware/generator_z/top/cgra_info.txt build/cgra_info_8x8.txt
-   CGRAGenerator/bin/cgra_info_analyzer.csh build/cgra_info_8x8.txt
-
@@ -92,20 +78,20 @@ serpent_only:
 core_tests:
    make clean_pnr
 #       # For verbose output add "SILENT=FALSE" to command line(s) below
-   make build/pointwise.correct.txt DELAY=0.0   GOLD=ignore
    make build/conv_1_2.correct.txt  DELAY=1,0   GOLD=ignore
    make build/conv_2_1.correct.txt  DELAY=10,0  GOLD=ignore
    make build/conv_3_1.correct.txt  DELAY=20,0  GOLD=ignore
    make build/conv_bw.correct.txt   DELAY=130,0 GOLD=ignore
+   make build/pointwise.correct.txt DELAY=0,0   GOLD=ignore

 serpent_tests:
    make clean_pnr
 #       # For verbose output add "SILENT=FALSE" to command line(s) below
-   make build/pointwise.correct.txt DELAY=0.0   GOLD=ignore PNR=serpent
+   make build/pointwise.correct.txt DELAY=0,0   GOLD=ignore PNR=serpent
    make build/conv_1_2.correct.txt  DELAY=1,0   GOLD=ignore PNR=serpent
    make build/conv_2_1.correct.txt  DELAY=10,0  GOLD=ignore PNR=serpent
    make build/conv_3_1.correct.txt  DELAY=20,0  GOLD=ignore PNR=serpent
-#  make build/conv_bw.correct.txt   DELAY=130,0 GOLD=ignore PNR=serpent
+   make build/conv_bw.correct.txt   DELAY=130,0 GOLD=ignore PNR=serpent
@@ -33,22 +33,8 @@ $(warning OUTPUT = "$(OUTPUT)")
 # Image being used
 IMAGE := default

-CGRA_SIZE := 8x8
+CGRA_SIZE := 16x16
 MEM_SWITCH := -newmem  # Don't really need this...riiight?
-ifeq ($(CGRA_SIZE), 4x4)
-   MEM_SWITCH := -oldmem -4x4
-endif
-
-ifeq ($(CGRA_SIZE), 8x8)
-#  MEM_SWITCH := -newmem -8x8
-   MEM_SWITCH := -newmem
-endif
-
-# should be the default!
-ifeq ($(CGRA_SIZE), 16x16)
-   MEM_SWITCH := -newmem
-endif
-

Do we want to include them?

cdonovick commented 6 years ago

@leonardt I am not sure what I am looking at. Regardless I don't think this is accurate. Did you pull before generating diffs?

For example

-   ./run.csh top_tb.cpp -hackmem           \

is not found in sixteen, fresh_sixteen, or master

leonardt commented 6 years ago

This is the diff I was looking at: https://github.com/StanfordAHA/CGRAFlow/pull/59/files

leonardt commented 6 years ago

Okay, figured it out, the diff shown by github was wrong/out-of-date because there was a conflict preventing the merge. I branched off sixteen with

git checkout sixteen
git pull
git checkout sixteen-master-merge
git fetch origin
git merge origin/master

I resolved the merge conflicts with https://github.com/StanfordAHA/CGRAFlow/commit/497450bdbd2220ba463b7daf750a74b67b52d3a7 and then I get a diff https://github.com/StanfordAHA/CGRAFlow/compare/master...sixteen-master-merge (click on files changed to see) that basically matches this PR, so this looks good to go

steveri commented 6 years ago

I see that the serpent tests have been removed from the .travis.yml for this branch... For completeness,, shouldn't we add them back? I think they are currently running correctly in the CGRAFlow master branch?

leonardt commented 6 years ago

Hi Steve, they were removed with the note # serpent currently broken I believe by @cdonovick (caleb, can you confirm?). I created a new branch and opened a PR to track it here https://github.com/StanfordAHA/CGRAFlow/pull/60 to see what the current status of serpent is with master

cdonovick commented 6 years ago

@steveri serpent is definitely broken - it doesn't handle features that were in coreir:dev (now master).

steveri commented 6 years ago

Oh, okay. Go ahead and check in (pull) this version that works then (with smt and no serpent). I will work in the background to bring serpent up to speed on a separate branch...