common.mk: Yosys compatible output can be generated by firtool without invoking SFC before MFC

[oharboe]

changelog:fixed

Related PRs / Issues:

Type of change:

• Bug fix
• New feature
• Other enhancement

Impact:

• RTL change
• Software change (RISC-V software)
• Build system change
• Other

Contributor Checklist:

• Did you set main as the base branch?
• Is this PR's title suitable for inclusion in the changelog and have you added a changelog:<topic> label?
• Did you state the type-of-change/impact?
• Did you delete any extraneous prints/debugging code?
• Did you mark the PR with a changelog: label?
• (If applicable) Did you update the conda .conda-lock.yml file if you updated the conda requirements file?
• (If applicable) Did you add documentation for the feature?
• (If applicable) Did you add a test demonstrating the PR?

• (If applicable) Did you mark the PR as Please Backport?

[abejgonzalez]

@nayiri-k is looking at this and testing it.

[abejgonzalez]

It looks like this flag was already added before:

chipyard/common.mk

Line 218 in bfa822a

ifeq (,$(ENABLE_YOSYS_FLOW))

.

[abejgonzalez]

Additionally,

chipyard/common.mk

Line 218 in bfa822a

ifeq (,$(ENABLE_YOSYS_FLOW))

needs to change so that the flag isn't added by default.

[nayiri-k]

diff --git a/common.mk b/common.mk
index f6bc19c1..50a6b531 100644
--- a/common.mk
+++ b/common.mk
@@ -28,7 +28,7 @@ EXTRA_SIM_CXXFLAGS   ?=
 EXTRA_SIM_LDFLAGS    ?=
 EXTRA_SIM_SOURCES    ?=
 EXTRA_SIM_REQS       ?=
-ENABLE_CUSTOM_FIRRTL_PASS += $(ENABLE_YOSYS_FLOW)
+# ENABLE_CUSTOM_FIRRTL_PASS += $(ENABLE_YOSYS_FLOW)
 
 ifneq ($(ASPECTS), )
        comma = ,
@@ -191,6 +191,12 @@ SFC_MFC_TARGETS = \
 
 SFC_REPL_SEQ_MEM = --infer-rw --repl-seq-mem -c:$(MODEL):-o:$(SFC_SMEMS_CONF)
 MFC_BASE_LOWERING_OPTIONS ?= emittedLineLength=2048,noAlwaysComb,disallowLocalVariables,verifLabels,locationInfoStyle=wrapInAtSquareBracket
+ifneq ($(ENABLE_YOSYS_FLOW),)
+       ifneq ($(MFC_BASE_LOWERING_OPTIONS),)
+               MFC_BASE_LOWERING_OPTIONS := $(MFC_BASE_LOWERING_OPTIONS),
+       endif
+       MFC_BASE_LOWERING_OPTIONS := $(MFC_BASE_LOWERING_OPTIONS)disallowPackedArrays
+endif
 
 # DOC include start: FirrtlCompiler
 # There are two possible cases for this step. In the first case, SFC
@@ -215,11 +221,12 @@ endif
 
 $(MFC_LOWERING_OPTIONS):
        mkdir -p $(dir $@)
-ifeq (,$(ENABLE_YOSYS_FLOW))
        echo "$(MFC_BASE_LOWERING_OPTIONS)" > $@
-else
-       echo "$(MFC_BASE_LOWERING_OPTIONS),disallowPackedArrays" > $@
-endif
+# ifeq (,$(ENABLE_YOSYS_FLOW))
+#      echo "$(MFC_BASE_LOWERING_OPTIONS)" > $@
+# else
+#      echo "$(MFC_BASE_LOWERING_OPTIONS),disallowPackedArrays" > $@
+# endif
 
 $(FINAL_ANNO_FILE): $(EXTRA_ANNO_FILE) $(SFC_EXTRA_ANNO_FILE) $(SFC_LEVEL)
        if [ $(shell cat $(SFC_LEVEL)) = low ]; then jq -s '[.[][]]' $(EXTRA_ANNO_FILE) $(SFC_EXTRA_ANNO_FILE) > $@; fi

[nayiri-k]

In the vlsi directory, I ran make buildfile tutorial=sky130-openroad which compiles a TinyRocketConfig, which gave the error: unsupported packed array expression. It seems that the disallowPackedArrays doesn't work in all cases.

[oharboe]

@nayiri-k Try with the latest firtool(not released yet, will be in firtool >1.59.0), should be fixed in llvm/circt#6402

[abejgonzalez]

Once FIRTOOL > 1.59.0 is released I'll update the feedstock and we can run CI on this to see what happens (probably after the holiday break I assume).

[oharboe]

@abejgonzalez Good news! 1.60.0 is out :-) https://github.com/llvm/circt/releases/tag/firtool-1.60.0

[abejgonzalez]

@Mergifyio rebase main

[mergify]

rebase main

❌ Pull request can't be updated with latest base branch changes

Mergify needs the author permission to update the base branch of the pull request.
@Pinata-Consulting needs to authorize modification on its head branch.

[abejgonzalez]

@Mergifyio copy main

[mergify]

copy main

✅ Pull request copies have been created

• #1690 common.mk: Yosys compatible output can be generated by firtool without invoking SFC before MFC (copy #1661) has been created for branch main

[abejgonzalez]

#1695 updates this