diff --git a/.claude/commands/bugfix.md b/.claude/commands/bugfix.md index 68f43b0c8..7834613ed 100644 --- a/.claude/commands/bugfix.md +++ b/.claude/commands/bugfix.md @@ -1374,6 +1374,18 @@ HDL method). A "simplification" that quietly moves an edge case is a second bug only affects scalac diagnostics; if a bad message survives that flag, stop suspecting the custom printer. +- **A tool-script printer that reads a SUB-DESIGN's parameter must resolve it through the + instantiation site.** `getConstData` under the default `Always` cache policy deliberately + leaves a sub-design's `DesignParam` an opaque `UnknownConst` (only a device-top parameter + folds), so `param.getConstData[...].toOption.get` on a vendor IP block's parameter is a + guaranteed `None.get` (`platforms/Test/runMain BlinkerNexys`, the Vivado `create_ip` script). + `getConstDataThroughParams` is the right query, and the two IP printers (Vivado, Quartus) are + twins of the kind described above: fix both through one helper (`ipParamData` in + `toolsCore/helpers.scala`). The printers had no test at all, which is how the shape survived; + `VendorIPPrinterSpec` now pins both scripts from a compiled design's `stagedDB`. Note the + printers list EVERY vendor IP block regardless of the target vendor, so a spec asserting on + the file list must select by path. + - **The Verilog printer prints arithmetic funcs bare and relies on the CONSUMER to size them — self-determined contexts break that contract, and the fix belongs in a STAGE, not the printer.** A carry-widened func (IR width exceeds its operands') is correct under an diff --git a/lib/src/main/scala/dfhdl/tools/toolsCore/QuartusPrime.scala b/lib/src/main/scala/dfhdl/tools/toolsCore/QuartusPrime.scala index 01e6db30c..46c7e595e 100644 --- a/lib/src/main/scala/dfhdl/tools/toolsCore/QuartusPrime.scala +++ b/lib/src/main/scala/dfhdl/tools/toolsCore/QuartusPrime.scala @@ -325,11 +325,11 @@ class QuartusPrimeIPPrinter(using val members = qsysIP.members(MemberView.Folded) val ipVersion = members.collectFirst { case param: DFVal.DesignParam if param.getName == "version" => - " " + param.getConstData[Option[String]].toOption.get.get + " " + param.ipParamData(ipName) }.getOrElse("") val ipParams = members.collect { case param: DFVal.DesignParam if param.getName != "version" => - s"set_instance_parameter_value $ipInstanceName {${param.getName}} {${param.getConstData[Option[Any]].toOption.get.get}}" + s"set_instance_parameter_value $ipInstanceName {${param.getName}} {${param.ipParamData(ipName)}}" }.mkString("\n") val ipExports = members.collect { case port @ DclPort() => diff --git a/lib/src/main/scala/dfhdl/tools/toolsCore/Vivado.scala b/lib/src/main/scala/dfhdl/tools/toolsCore/Vivado.scala index 8f5468e61..ff058561d 100644 --- a/lib/src/main/scala/dfhdl/tools/toolsCore/Vivado.scala +++ b/lib/src/main/scala/dfhdl/tools/toolsCore/Vivado.scala @@ -331,7 +331,7 @@ class VivadoIPPrinter(using val members = vivadoIP.members(MemberView.Folded) val ipVersion = members.collectFirst { case param: DFVal.DesignParam if param.getName == "version" => - val version = param.getConstData[Option[String]].toOption.get.get + val version = param.ipParamData(ipName).asInstanceOf[String] if (version.nonEmpty) Some(" -version " + version) else None }.flatten.getOrElse("") @@ -339,8 +339,7 @@ class VivadoIPPrinter(using // Each DFVal.DesignParam except "version" becomes a CONFIG. {value} line val ipConfigParams = members.collect { case param: DFVal.DesignParam if param.getName != "version" => - val value = param.getConstData[Option[Any]].toOption.get.get - s"CONFIG.${param.getName} {$value}" + s"CONFIG.${param.getName} {${param.ipParamData(ipName)}}" } val ipConfigBlock = if ipConfigParams.nonEmpty then diff --git a/lib/src/main/scala/dfhdl/tools/toolsCore/helpers.scala b/lib/src/main/scala/dfhdl/tools/toolsCore/helpers.scala index d6c0ca143..9dcd8e6db 100644 --- a/lib/src/main/scala/dfhdl/tools/toolsCore/helpers.scala +++ b/lib/src/main/scala/dfhdl/tools/toolsCore/helpers.scala @@ -4,3 +4,20 @@ extension (flagVal: Boolean) def toFlag(flagName: String): String = if (flagVal) flagName else "" + +extension (param: dfhdl.compiler.ir.DFVal.DesignParam) + /** The applied data of a vendor IP block's parameter, for emitting the IP generation script. + * + * An IP block is a sub-design, so its parameter values live at its instantiation site and only + * `getConstDataThroughParams` resolves them; the default (`Always`) cache policy deliberately + * keeps a sub-design parameter opaque (`UnknownConst`). The data itself is `Option`-shaped + * (`None` is the bubble value), which an IP script cannot express either, so both cases raise an + * error naming the IP and the parameter. + */ + def ipParamData(ipName: String)(using dfhdl.compiler.ir.MemberGetSet): Any = + param.getConstDataThroughParams[Option[Any]].flatten.getOrElse( + throw new IllegalArgumentException( + s"The IP `$ipName` parameter `${param.getName}` has no constant value that can be resolved for the IP generation script." + ) + ) +end extension diff --git a/lib/src/test/scala/VendorIPPrinterSpec.scala b/lib/src/test/scala/VendorIPPrinterSpec.scala new file mode 100644 index 000000000..9888229a4 --- /dev/null +++ b/lib/src/test/scala/VendorIPPrinterSpec.scala @@ -0,0 +1,65 @@ +package dfhdl +import munit.* +import dfhdl.hw.annotation.top +import dfhdl.compiler.ir.MemberGetSet +import dfhdl.tools.toolsCore.{VivadoIPPrinter, QuartusPrimeIPPrinter} + +// The vendor IP generation scripts (Vivado `create_ip` tcl, Quartus qsys tcl) are emitted from the +// IP block's design parameters. An IP block is a sub-design, so the values it was instantiated +// with live at the instantiation site and must be resolved through it; asking the parameter +// under the default cache policy leaves it opaque and used to crash the printer (`None.get`). +class VendorIPPrinterSpec extends FunSuite: + class VivadoCounter( + val WIDTH: Int <> CONST = 8, + val CLK_PORT: String <> CONST = "clk", + val ENABLE: Boolean <> CONST = false, + val version: String <> CONST = "" + ) extends EDBlackBox.VivadoIP: + val clk = Bit <> IN + val cnt = Bits(WIDTH) <> OUT + + class QsysCounter( + val WIDTH: Int <> CONST = 8, + val CLK_PORT: String <> CONST = "clk", + val version: String <> CONST = "1.0" + ) extends EDBlackBox.QsysIP: + val clk = Bit <> IN + val cnt = Bits(WIDTH) <> OUT + + @top(false) class Top extends EDDesign: + val clk = Bit <> IN + val cntV = Bits(16) <> OUT + val cntQ = Bits(12) <> OUT + val vivadoCounter = new VivadoCounter(WIDTH = 16, CLK_PORT = "clock", ENABLE = true) + val qsysCounter = new QsysCounter(WIDTH = 12, version = "2.5") + vivadoCounter.clk <> clk + cntV <> vivadoCounter.cnt + qsysCounter.clk <> clk + cntQ <> qsysCounter.cnt + + test("vendor IP scripts carry the applied parameter values"): + val cd = Top().compile + given MemberGetSet = cd.stagedDB.getSet + val vivadoTcl = new VivadoIPPrinter().getSourceFiles + .find(_.path == "ips/VivadoCounter.tcl").get.contents + assertNoDiff( + vivadoTcl, + """|create_ip -name VivadoCounter -module_name VivadoCounter + |set_property -dict [list \ + | CONFIG.WIDTH {16} \ + | CONFIG.CLK_PORT {clock} \ + | CONFIG.ENABLE {true} \ + |] [get_ips VivadoCounter] + |""".stripMargin + ) + val qsysTcl = new QuartusPrimeIPPrinter().getSourceFiles + .find(_.path == "ips/QsysCounter.tcl").get.contents + assert(qsysTcl.contains("QsysCounter 2.5"), qsysTcl) + assert( + qsysTcl.contains( + """|set_instance_parameter_value QsysCounter_inst {WIDTH} {12} + |set_instance_parameter_value QsysCounter_inst {CLK_PORT} {clk}""".stripMargin + ), + qsysTcl + ) +end VendorIPPrinterSpec diff --git a/plugin/src/main/scala/plugin/MetaContextGenPhase.scala b/plugin/src/main/scala/plugin/MetaContextGenPhase.scala index bde30fe71..15500033e 100755 --- a/plugin/src/main/scala/plugin/MetaContextGenPhase.scala +++ b/plugin/src/main/scala/plugin/MetaContextGenPhase.scala @@ -46,7 +46,7 @@ class MetaContextGenPhase(setting: Setting) extends CommonPhase: var inlinedUserPosStack = List.empty[util.SrcPos] private def isUserSourced(tree: Tree)(using Context): Boolean = - tree.span.exists && tree.srcPos.startPos.source == ctx.compilationUnit.source + tree.span.exists && tree.srcPos.startPos.source.path == ctx.compilationUnit.source.path extension (tree: ValOrDefDef)(using Context) def needsNewContext: Boolean = @@ -209,7 +209,7 @@ class MetaContextGenPhase(setting: Setting) extends CommonPhase: val pos = if ( metaInfo.nameOpt.isEmpty && argTree.isProxyContext && - metaInfo.srcPos.startPos.source != ctx.compilationUnit.source + metaInfo.srcPos.startPos.source.path != ctx.compilationUnit.source.path ) inlinedUserPosStack.headOption .orElse(enclosingUserSrcPos) @@ -258,7 +258,7 @@ class MetaContextGenPhase(setting: Setting) extends CommonPhase: // macro-synthesized apply only carries macro-internal positions. private def enclosingUserSrcPos(using Context): Option[util.SrcPos] = applyStack.collectFirst { - case a if a.span.exists && a.srcPos.startPos.source == ctx.compilationUnit.source => + case a if a.span.exists && a.srcPos.startPos.source.path == ctx.compilationUnit.source.path => a.srcPos } diff --git a/plugin/src/main/scala/plugin/PluginTestPhase.scala b/plugin/src/main/scala/plugin/PluginTestPhase.scala index d3fe000d5..5c2895828 100644 --- a/plugin/src/main/scala/plugin/PluginTestPhase.scala +++ b/plugin/src/main/scala/plugin/PluginTestPhase.scala @@ -123,9 +123,13 @@ class PluginTestPhase(setting: Setting) extends CommonPhase: val fullCode = "import dfhdl.*\n" + code val source2 = SourceFile.virtual(unitName, fullCode) - // tested strings must not be rewritten by `-rewrite` + // tested strings must not be rewritten by `-rewrite`; the setting's own default (`None` + // before Scala 3.10, `false` from 3.10 on) keeps this independent of its value type val noRewriteSettings = - ctx.settings.rewrite.updateIn(ctx.settingsState.reinitializedCopy(), None) + ctx.settings.rewrite.updateIn( + ctx.settingsState.reinitializedCopy(), + ctx.settings.rewrite.default + ) class MegaPhaseWithCustomPhaseId(miniPhases: Array[MiniPhase], startId: Int, endId: Int) extends MegaPhase(miniPhases):