f4pga-arch-defs
f4pga-arch-defs copied to clipboard
Move xc7 yosys script contents into reusable tcl functions
Instead of calling python from the bash script wrapper, call it directly from conv.tcl
Signed-off-by: Olof Kindgren [email protected]
Forgot to mention, this is kind of a first step in my objective to eventually let Edalize call the individual EDA tools directly instead of going through the shell script wrappers.
Replaces https://github.com/SymbiFlow/symbiflow-arch-defs/pull/1461
Ok, I redid the PR. Updating topic.
The new PR moves most of the code out of synth.tcl and conv.tcl into functions, so that these functions can be called by something else (e.g. Edalize) directly instead of going through symbiflow_synth. This means that the code is still backwards-compatible.
A few notes:
- The code from synth.tcl moved into new file synth_functions.tcl. Perhaps extending utils.tcl would make more sense?
- I tried to create functions in synth_functions.tcl that grouped together commands into what I believed was a full function. This might be very wrong though as I don't fully understand all the logic in synth.tcl. I could move stuff around or simply have one big function that does all of synth.tcl
- I reduced the number of env vars as much as possible and only made $SHARE_DIR_PATH and $TOP required. The other env vars were effectively hard coded anyway. I did keep USE_ROI and USE_LUT_CONSTANTS though as those felt more like global debug flags. But again, I'm not sure I got that right
Updated now with restored TECHMAP_PATH and only calling python3 directly if there is no PYTHON3 env var
One thing. Do we need to register the new synth_functions.tcl in some CMake script or so to make sure it is installed? And do we want to move the synth_functions contents into utils.tcl instead or is it fine to have a new file for this?
@olofk Good question, I missed this one actually. So IMO it might be ok to have all the synth functions alongside with the ones in the already present utils.tcl file, which is already getting installed along with the other scripts.
I added all the functions to utils.tcl and updated the PR. It feels like they are similar to the ones already found there so I agree it make sense to have them all there
I see that CI failed. Is this the issue? [TCL: yosys -import] Command name collision: found pre-existing command wbfliERROR: TCL interpreter returned an error: can't read "::env(TOP)": no such variable`
Think it might be. Force-pushing with this change
diff --git a/xc/xc7/yosys/synth.tcl b/xc/xc7/yosys/synth.tcl
index 55592d92..66ab227d 100644
--- a/xc/xc7/yosys/synth.tcl
+++ b/xc/xc7/yosys/synth.tcl
@@ -11,7 +11,7 @@ yosys -import
source [file join [file normalize [info script]] .. utils.tcl]
-if { [info exists $::env(TOP)]} {
+if { [info exists ::env(TOP)]} {
set top $::env(TOP)
} else {
set top ""
I could also restore the env vars in synth.tcl instead. As long as we can have the functions in utils.tcl free from env vars that solves my original issue. That would make the diff smaller. Should I do it like that?
Updated now to restore the env vars
@olofk Can you please rebase on top of master, as there were some fixes to CI? I believe we can merge once everything goes green
Done. Let's hope for the CI to say yes
Stupid CI found another issue with my code. Pushed a new version now
Oh, come on! Failed again? This time it must work
Aarrrghh!!! Seems like there's some cmake scripts that needs updating. I have no idea what I'm doing anymore
Ok, fuck it. I give up with the cmake stuff and simplified the PR instead so that it doesn't touch symbiflow_synth at all. This can't possibly go wrong
Yes! All green. Good to go now @acomodi ?
@olofk Great! All CIs now are green (I forgot to run kokoro as well which is still used to generate and push new device databases)
Given that there were some other merges in between, it would be good to rebase this PR on top of main and than merge
@acomodi Rebased and repushed