OpenROAD-flow-scripts icon indicating copy to clipboard operation
OpenROAD-flow-scripts copied to clipboard

asap7 ADDER_MAP_FILE is identical for ASAP7_USELVT=1 and ASAP7_USELVT=0

Open oharboe opened this issue 9 months ago • 5 comments

Subject

[Flow] for any util, flow Makefile, or flow script issues.

Describe the bug

This is as intended, I think:

https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/blob/4004072bd74aa01fd60a76011bf64bca3c4e74f6/flow/platforms/asap7/config.mk#L83

$ make DESIGN_CONFIG=designs/asap7/aes/config.mk print-ADDER_MAP_FILE
ADDER_MAP_FILE = .../platforms/asap7/yoSys/cells_adders_R.v

I believe it was the intention to get cells_adders_L.v with ASAP7_USELVT=1

https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/blob/4004072bd74aa01fd60a76011bf64bca3c4e74f6/flow/platforms/asap7/config.mk#L178

$ make DESIGN_CONFIG=designs/asap7/aes_lvt/config.mk print-ADDER_MAP_FILE
ADDER_MAP_FILE = .../platforms/asap7/yoSys/cells_adders_R.v

For anyone debugging this, in addition to the make print-FOO tool, I find that it is useful to run thish command on master and on my branch:

make DESIGN_CONFIG=designs/asap7/aes_lvt/config.mk vars

This generates a vars.sh file in canonical form with all the variables the makefiles has. This vars.sh file can be compared against master and my branch:

$ diff vars.sh ../../OpenROAD-flow-scripts/flow/vars.sh 
5c5
< export ADDER_MAP_FILE="${FLOW_HOME}/platforms/asap7/yoSys/cells_adders_R.v"
---
> export ADDER_MAP_FILE="${FLOW_HOME}/platforms/asap7/yoSys/cells_adders_L.v"
40c40
...

Expected Behavior

Different ADDER_MAP_FILE values

Environment

ORFS: v3.0-2686-g4004072b

To Reproduce

See above

Relevant log output


Screenshots

No response

Additional Context

No response

oharboe avatar Mar 23 '25 06:03 oharboe

It would probably be a good idea to check that everything that should be different is different+has the correct value and everything that should be the same is the same...

$ make DESIGN_CONFIG=designs/asap7/aes/config.mk ASAP7_USELVT=1 vars
.../flow/util/generate-vars.sh vars
$ cp vars.sh vars-lvt.sh
$ make DESIGN_CONFIG=designs/asap7/aes/config.mk ASAP7_USELVT= vars
.../flow/util/generate-vars.sh vars
$ diff vars.sh  vars-lvt.sh

Also, I think it would be a good idea to change the variable in platforms/asap7/config.mk to be a mutually exclusive variable, consistent with e.g. CORNER:

ASAP7_USE=LVT/SLVT/RVT, where RVT is default.

oharboe avatar Mar 23 '25 08:03 oharboe

I think the problem is lvt uses ?= in some places which fails to override the rvt ?= settings. I agree it would be good to just have separate blocks which would avoid this and allow ?= to be used throughout.

maliberty avatar Mar 23 '25 15:03 maliberty

I think the problem is lvt uses ?= in some places which fails to override the rvt ?= settings. I agree it would be good to just have separate blocks which would avoid this and allow ?= to be used throughout.

Yep... I have developed a deeper understanding for defaults, late evaluation, command line and env var subtleties in make than any man should.

oharboe avatar Mar 23 '25 16:03 oharboe

Is a generic pattern with PLATFORM_VARIANT useful/common?

oharboe avatar Mar 23 '25 16:03 oharboe

Raised https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/pull/2996 for review. LMK if it aligns with what you both were thinking.

jeffng-or avatar Mar 24 '25 20:03 jeffng-or