search

GaloisInc / BESSPIN-CloudGFE

The AWS cloud deployment of the BESSPIN GFE platform.
Apache License 2.0
2 stars 2 forks source link

AWSteria Cheri-Flute multiple clocks #112

Closed charlie-bluespec closed 4 years ago

charlie-bluespec commented 4 years ago

@joestoy to fill in details and status

joestoy commented 4 years ago

Done, both for verilator simulation and AWS, but still needs merging with @rsnikhil's virtio stuff and Alexcadre's amendments to move the tag controller nearer top level, when they are ready. Does not quite meet timing for AWS at 100MHz for the core; but the bitstream works on the FPGA, and it's probably not worth working more on timing closure until the logic (particularly the second item mentioned above) has stabilized.

jrtc27 commented 4 years ago

For Connectal I added the following overrides on top of aws-fpga's strategy_TIMING.tcl (see hw/scripts/strategy_OVERRIDES.tcl, and hw/scripts/Include_Common.mk which specifies TIMING):

set synth_options   "-no_lc -shreg_min_size 10 -control_set_opt_threshold 16 $synth_uram_option"
set synth_directive "AlternateRoutability"

set place_directive "AltSpreadLogic_medium"

set route_directive "AggressiveExplore"

The synt/place options come from CONGESTION. The route_directive forces Vivado to rip-up and reroute multiple times and after about 3 attempts (and sometimes post-route physical optimisation) passes timing on Connectal. Given the cores themselves are extremely similar between the two, and the shell is the same big blob, I would expect that to give similar results for AWSteria. You're right there's probably not much point spending a lot of time to close timing, but you could try this patch (completely untested, may have silly bugs) if you have a spare moment to kick off a build:

diff --git a/AWSteria/Doc/Makefile_AFI_build.mk b/AWSteria/Doc/Makefile_AFI_build.mk
index bea4ed3..b5cef87 100644
--- a/AWSteria/Doc/Makefile_AFI_build.mk
+++ b/AWSteria/Doc/Makefile_AFI_build.mk
@@ -156,7 +156,9 @@ Step_2a_Setup_Email:
 # The following are options to script 'aws_build_dcp_from_cl.sh'
 # Adjust them per your requirements

-BUILD_DCP_FLAGS =                                # Default Clock Group A Recipe A0 (125 MHz)
+BUILD_DCP_FLAGS  = -clock_recipe_a A1            # Clock Group A Recipe A1 (250 MHz)
+BUILD_DCP_FLAGS += -clock_recipe_b B5            # Clock Group B Recipe B5 (100 MHz)
+BUILD_DCP_FLAGS += -strategy CUSTOM              # Use our custom strategy
 BUILD_DCP_FLAGS += -notify                       # Notify completion by email
 BUILD_DCP_FLAGS += -ignore_memory_requirement    # avoid ERROR: your instance has less mem than is necessary

diff --git a/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/aws_build_dcp_from_cl.sh b/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/aws_build_dcp_from_cl.sh
index e8d2023..ef2d7ac 100755
--- a/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/aws_build_dcp_from_cl.sh
+++ b/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/aws_build_dcp_from_cl.sh
@@ -18,7 +18,7 @@
 # Usage help
 function usage
 {
-    echo "usage: aws_build_dcp_from_cl.sh [ [-script <vivado_script>] | [-strategy BASIC | DEFAULT | EXPLORE | TIMING | CONGESTION] [-clock_recipe_a A0 | A1 | A2] [-clock_recipe_b B0 | B1 | B2 | B3 | B4 | B5] [-clock_recipe_c C0 | C1 | C2 | C3] [-uram_option 2 | 3 | 4] [-vdefine macro1,macro2,macro3,.....,macrox] -foreground] [-notify] | [-h] | [-H] | [-help] ]"
+    echo "usage: aws_build_dcp_from_cl.sh [ [-script <vivado_script>] | [-strategy BASIC | DEFAULT | EXPLORE | TIMING | CONGESTION | CUSTOM] [-clock_recipe_a A0 | A1 | A2] [-clock_recipe_b B0 | B1 | B2 | B3 | B4 | B5] [-clock_recipe_c C0 | C1 | C2 | C3] [-uram_option 2 | 3 | 4] [-vdefine macro1,macro2,macro3,.....,macrox] -foreground] [-notify] | [-h] | [-H] | [-help] ]"
     echo " "
     echo "By default the build is run in the background using nohup so that the"
     echo "process will not be terminated if the terminal window is closed."
@@ -34,9 +34,9 @@ function usage
 }

 # Default arguments for script and strategy
-strategy=TIMING
-clock_recipe_a=A1
-clock_recipe_b=B5
+strategy=DEFAULT
+clock_recipe_a=A0
+clock_recipe_b=B0
 clock_recipe_c=C0
 vivado_script="create_dcp_from_cl.tcl"
 foreground=0
@@ -117,8 +117,8 @@ fi

 # Check that strategy is valid
 shopt -s extglob
-if [[ $strategy != @(BASIC|DEFAULT|EXPLORE|TIMING|CONGESTION) ]]; then
-  err_msg "$strategy isn't a valid strategy. Valid strategies are BASIC, DEFAULT, EXPLORE, TIMING and CONGESTION."
+if [[ $strategy != @(BASIC|DEFAULT|EXPLORE|TIMING|CONGESTION|CUSTOM) ]]; then
+  err_msg "$strategy isn't a valid strategy. Valid strategies are BASIC, DEFAULT, EXPLORE, TIMING, CONGESTION and CUSTOM."
   exit 1
 fi

diff --git a/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/create_dcp_from_cl.tcl b/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/create_dcp_from_cl.tcl
index 1cdaca4..832d753 100644
--- a/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/create_dcp_from_cl.tcl
+++ b/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/create_dcp_from_cl.tcl
@@ -182,6 +182,10 @@ switch $strategy {
         puts "DEFAULT strategy."
         source $HDK_SHELL_DIR/build/scripts/strategy_DEFAULT.tcl
     }
+    "CUSTOM" {
+        puts "CUSTOM strategy."
+        source $CL_DIR/build/scripts/strategy_CUSTOM.tcl
+    }
     default {
         puts "$strategy is NOT a valid strategy. Defaulting to strategy DEFAULT."
         source $HDK_SHELL_DIR/build/scripts/strategy_DEFAULT.tcl
diff --git a/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/strategy_CUSTOM.tcl b/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/strategy_CUSTOM.tcl
index e69de29..b8b5451 100644
--- a/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/strategy_CUSTOM.tcl
+++ b/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/strategy_CUSTOM.tcl
@@ -0,0 +1,10 @@
+source $HDK_SHELL_DIR/build/scripts/strategy_TIMING.tcl
+
+# Use CONGESTION place and synth options instead
+set synth_options   "-no_lc -shreg_min_size 10 -control_set_opt_threshold 16 $synth_uram_option"
+set synth_directive "AlternateRoutability"
+
+set place_directive "AltSpreadLogic_medium"
+
+# We need multiple rip-up and reroute passes
+set route_directive "AggressiveExplore"

As part of this I cleaned up your changes. IMO it's best practice to not change defaults within the script and instead be explicit in the AWSteria Makefile as to the changes you want. We could even consider proposing adding the CUSTOM strategy to upstream aws-fpga so that future CL templates would work out of the box for AWSteria.

joestoy commented 4 years ago

I aqree on best practice, I think. Currently in AWSteria I'm using the standard A1 and B5 recipes for 250MHz and 100MHz, and the standard TIMING strategy. That meets timing (after enormous effort) for vanilla Flute, but not quite for CHERI Flute. (I agree I'm currently specifying that in the shell script, rather than the Makefile itself.) I'll give your custom strategy a whirl.

(Actually, I'm a bit suspcious about doing too much in Makefiles. I guess it's fine to specify synthesis strategy there; but I don't like it when the content of the design itself depends on complicated conditionals defining paths in the Makefile. I like my proofs of correctness to be done wrt just one programming language, let alone a language which contains awful things like "?=" iwhich flies in the face of all good scoping discipline. But this is probably best discussed over a drink sometime, if we get the chance ever, and not here!)

jrtc27 commented 4 years ago

Thankfully this is = then +=, no ?= :) but that did make me spot that I used = rather than +=; patch updated.

jrtc27 commented 4 years ago

Using set place_direct "AltSpreadLogic_low" instead of AltSpreadLogic_medium seems to give slightly quicker (and thus perhaps more reliable?) convergence towards a design that meets timing for Connectal (both vanilla GFE and CHERI). Presumably because although we need to avoid congestion lest the router have to take long detours we also have quite deep logic in Flute and so spreading the logic out too much creates a lot of work needing to be undone later on, and the less-aggressive AltSpreadLogic_low is a better middle-ground that doesn't sacrifice path lengths too much in the name of congestion. It does still need the AggressiveExplore route directive though.

I would therefore suggest you try the following strategy_CUSTOM.tcl instead if you haven't had success already:

--- a/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/strategy_CUSTOM.tcl
+++ b/AWSteria/developer_designs/cl_BSV_WindSoC/build/scripts/strategy_CUSTOM.tcl
@@ -0,0 +1,12 @@
+source $HDK_SHELL_DIR/build/scripts/strategy_TIMING.tcl
+
+# Use CONGESTION synth options instead
+set synth_options   "-no_lc -shreg_min_size 10 -control_set_opt_threshold 16 $synth_uram_option"
+set synth_directive "AlternateRoutability"
+
+# Use weaker variant of CONGESTION place directive instead to balance
+# avoiding congestion with avoiding long paths (we have deep logic)
+set place_directive "AltSpreadLogic_low"
+
+# We need multiple rip-up and reroute passes
+set route_directive "AggressiveExplore"
charlie-bluespec commented 4 years ago

Complete. Alexandre will have to add multiple clocks as he tracks Nikhil's virtio work on vanilla Bluespec P2.