From 99316dd99e29e7032de13f5d7d3b32a336728d83 Mon Sep 17 00:00:00 2001 From: Oron Date: Tue, 25 Aug 2026 00:32:19 +0300 Subject: [PATCH 1/6] latch prevention: an RT body's conditional lowers to a combinational process DropLocalDcls Rule 5 recognized a combinational scope only as a process(all) block, but the stage really runs pre-lowering (the ExplicitState dependency edge pulls it in before ToED, and the BackendPrepStage slot is then deduped), so a variable hoisted out of a conditional sitting directly in an RT domain body received no don't-care default and inferred a latch in the generated always_comb. A non-process RT domain body (design or domain block) is combinational by construction and is wrapped in process(all) by ToED, so it now receives the same default. A DF domain body stays excluded: ExplicitState resolves an undriven path there to implied state, which a per-activation default would break. Co-Authored-By: Claude Fable 5 --- .claude/commands/new-stage.md | 23 +++-- .../dfhdl/compiler/stages/DropLocalDcls.scala | 56 ++++++++--- .../scala/StagesSpec/DropLocalDclsSpec.scala | 94 +++++++++++++++++++ 3 files changed, 156 insertions(+), 17 deletions(-) diff --git a/.claude/commands/new-stage.md b/.claude/commands/new-stage.md index dad06b28e..1cabc56c0 100644 --- a/.claude/commands/new-stage.md +++ b/.claude/commands/new-stage.md @@ -775,12 +775,23 @@ declaration escaped from. Two mechanics worth reusing: - `Move(anchor, Before)` + `Add(anchor, Before)` on the SAME anchor merge, with the added members appended AFTER the moved ones (list the Move entries first) — that places the default right after the relocated declaration under VHDL without a second phase. -- Scope such compensation by the exact semantic trigger, not by the move: only - `Sensitivity.All` processes (`process(all)` is ED-only; an RT `process` has - `Sensitivity.List(Nil)`), only no-init declarations (an init declares deliberate state - retention), and only genuinely conditional scopes (`if`/`match` branch or `while` body; a - `for` body runs a static range, and a clocked guard-style process must NOT get a - process-level default, which would sit outside the clock guard). +- Scope such compensation by the exact semantic trigger, not by the move: only scopes that + lower to a combinational process — `Sensitivity.All` processes (`process(all)` is ED-only; + an RT `process` has `Sensitivity.List(Nil)`) AND non-process RT domain bodies (a design or + domain block, which ToED later wraps in `process(all)`; a DF body is excluded because + ExplicitState resolves an undriven path to implied state, which a per-activation default + would break), only no-init declarations (an init declares deliberate state retention), and + only genuinely conditional scopes (`if`/`match` branch or `while` body; a `for` body runs a + static range, and a clocked guard-style process must NOT get a process-level default, which + would sit outside the clock guard). +- A stage's REAL run position may be earlier than its `BackendPrepStage` slot: a dependency + edge (here `ExplicitState -> DropLocalDcls`) pulls it into the pre-lowering pipeline, and + `StageRunner` then DEDUPES the late slot, so the stage never re-runs post-ToED unless + nullified. Consequently pre-lowering domain shapes (RT/DF design bodies, domain blocks) are + legitimate inputs the stage must handle, and a rule keyed on "what block holds this scope" + must model what that block LOWERS TO, not only the post-ToED process forms. Read the + `Running stage` sequence of a full `--log trace` run to learn where a stage actually fires + before trusting the bundle order. ### Pattern 3 — Construct new members with `MetaDesign` ```scala diff --git a/compiler/stages/src/main/scala/dfhdl/compiler/stages/DropLocalDcls.scala b/compiler/stages/src/main/scala/dfhdl/compiler/stages/DropLocalDcls.scala index 016f40cad..f5ff9fdc3 100644 --- a/compiler/stages/src/main/scala/dfhdl/compiler/stages/DropLocalDcls.scala +++ b/compiler/stages/src/main/scala/dfhdl/compiler/stages/DropLocalDcls.scala @@ -172,10 +172,36 @@ import scala.annotation.tailrec * tmp := a + b * res := tmp * }}} + * The same applies to a variable lifted out of a conditional sitting directly in an RT domain + * body (a design or domain block, outside any process): such a body is combinational by + * construction and is later lowered to a `process(all)` by ToED, at which point the lifted + * variable is only driven on some of the process paths and infers the same latch: + * {{{ + * // Before: tmp declared (without init) inside an if in an RT design body + * class ID extends RTDesign: + * res := a + * if (sel) + * val tmp = UInt(8) <> VAR + * tmp := a + b + * res := tmp + * + * // After: moved to design level, don't-care default before the if + * class ID extends RTDesign: + * val tmp = UInt(8) <> VAR + * res := a + * tmp := d"8'?" + * if (sel) + * tmp := a + b + * res := tmp + * }}} * The default applies only when all of the following hold, since otherwise the lift does not * create an incompletely-driven combinational variable: - * - the enclosing process is `process(all)` (an explicit sensitivity list keeps the target - * language's own semantics, and a clocked process infers registers, not latches) + * - the escaped scope lowers to a combinational process: the enclosing process is + * `process(all)` (an explicit sensitivity list keeps the target language's own semantics, + * and a clocked process infers registers, not latches), or the conditional sits directly in + * an RT domain body, which ToED later wraps in a `process(all)`. A DF domain body is + * excluded: ExplicitState resolves an undriven path to implied state, which a + * per-activation default would break. * - the declaration has no init (an init declares deliberate state retention, which a * per-activation default would break) * - the escaped scope is genuinely conditional: an `if`/`match` branch or a `while` body @@ -219,17 +245,25 @@ case object DropLocalDcls extends HierarchyStage: dclVal match case dcl @ DclVar() if !dcl.isReg && dcl.initRefList.isEmpty && insideConditional(dcl) => val (anchor, scopeBlock) = climbToScope(dcl) - scopeBlock match + val combinationalScope = scopeBlock match + // an explicit sensitivity list keeps the target language's own semantics, and a + // clocked process infers registers, not latches case pb: ProcessBlock => pb.sensitivity match - case ProcessBlock.Sensitivity.All => - val dsn = new MetaDesign(anchor, Patch.Add.Config.Before): - dcl.asVarAny.:=( - dfhdl.core.Bubble.constValOf(new dfhdl.core.DFType(dcl.dfType), named = false) - )(using dfc.setMetaAnon(dcl.meta.position)) - Some(dsn.patch) - case _ => None - case _ => None + case ProcessBlock.Sensitivity.All => true + case _ => false + // an RT domain body outside a process (a design or domain block) is combinational by + // construction and is lowered to a `process(all)` by ToED, so a variable lifted there + // creates the same latch. A DF domain body is excluded: ExplicitState resolves an + // undriven path to implied state, which a per-activation default would break. + case _ => dcl.isInRTDomain + if combinationalScope then + val dsn = new MetaDesign(anchor, Patch.Add.Config.Before): + dcl.asVarAny.:=( + dfhdl.core.Bubble.constValOf(new dfhdl.core.DFType(dcl.dfType), named = false) + )(using dfc.setMetaAnon(dcl.meta.position)) + Some(dsn.patch) + else None case _ => None // Whether the member's lexical scope is conditioned: an enclosing block between the member and diff --git a/compiler/stages/src/test/scala/StagesSpec/DropLocalDclsSpec.scala b/compiler/stages/src/test/scala/StagesSpec/DropLocalDclsSpec.scala index bd675e2f4..97bfbfcb6 100644 --- a/compiler/stages/src/test/scala/StagesSpec/DropLocalDclsSpec.scala +++ b/compiler/stages/src/test/scala/StagesSpec/DropLocalDclsSpec.scala @@ -359,4 +359,98 @@ class DropLocalDclsSpec extends StageSpec: |end ID |""".stripMargin ) + + test("RTDomain combinational latch prevention"): + class Foo extends RTDesign: + val x = UInt(8) <> IN + val y = Bits(4) <> OUT + val b = Bit <> IN + if (b) + val temp = UInt(8) <> VAR + temp := x + d"8'1" + y := temp.bits(3, 0) + else y := h"0" + end if + end Foo + val foo = (new Foo).dropLocalDcls + assertCodeString( + foo, + """|class Foo extends RTDesign: + | val x = UInt(8) <> IN + | val y = Bits(4) <> OUT + | val b = Bit <> IN + | val temp = UInt(8) <> VAR + | temp := ? + | if (b) + | temp := x + d"8'1" + | y := temp.bits(3, 0) + | else y := h"0" + | end if + |end Foo + |""".stripMargin + ) + + test("RTDomain block combinational latch prevention"): + class Foo extends EDDesign: + val x = UInt(8) <> IN + val y = Bits(4) <> OUT + val b = Bit <> IN + val dmn = new RTDomain: + if (b) + val temp = UInt(8) <> VAR + temp := x + d"8'1" + y := temp.bits(3, 0) + else y := h"0" + end if + end Foo + val foo = (new Foo).dropLocalDcls + assertCodeString( + foo, + """|class Foo extends EDDesign: + | val x = UInt(8) <> IN + | val y = Bits(4) <> OUT + | val b = Bit <> IN + | val dmn = new RTDomain: + | val temp = UInt(8) <> VAR + | temp := ? + | if (b) + | temp := x + d"8'1" + | y := temp.bits(3, 0) + | else y := h"0" + | end if + | end dmn + |end Foo + |""".stripMargin + ) + + // A DF domain body gets no don't-care default: ExplicitState resolves an undriven path to + // implied state, which a per-activation default would break. + test("No combinational defaults in DF domain bodies"): + class Foo extends DFDesign: + val x = UInt(8) <> IN + val y = Bits(4) <> OUT + val b = Bit <> IN + if (b) + val temp = UInt(8) <> VAR + temp := x + d"8'1" + y := temp.bits(3, 0) + else y := h"0" + end if + end Foo + val foo = (new Foo).dropLocalDcls + assertCodeString( + foo, + """|class Foo extends DFDesign: + | val x = UInt(8) <> IN + | val y = Bits(4) <> OUT + | val b = Bit <> IN + | val temp = UInt(8) <> VAR + | if (b) + | temp := x + d"8'1" + | y := temp.bits(3, 0) + | else y := h"0" + | end if + |end Foo + |""".stripMargin + ) end DropLocalDclsSpec From 53c55645add0ea6f3f51d1d5f338f0d9113055fc Mon Sep 17 00:00:00 2001 From: Oron Date: Tue, 25 Aug 2026 01:32:24 +0300 Subject: [PATCH 2/6] lint waivers: @unused.quiet takes an optional bit range, minted on compiler-named selections A value the compiler names for backend syntax (e.g. NamedVerilogSelection naming a bit-selection prefix) becomes a declared signal, so verilator's UNUSEDSIGNAL reports its unread bits, a warning the user's anonymous source has no handle on. Unused.Quiet now carries an optional bit range, and NamedAliases annotates the bits none of the group's readers statically select, one annotation per contiguous range. The verilator config printer turns these into bit-precise waivers, joining a signal's ranges the way verilator prints them in one message ('x'[7:5,2:0]). Being a printed annotation rather than a tag, a printed stage output re-elaborates to the same waivers (print-safe fix-point). Co-Authored-By: Claude Fable 5 --- .claude/commands/new-stage.md | 7 ++ .../scala/dfhdl/compiler/ir/annotation.scala | 20 +++++- .../dfhdl/compiler/analysis/DBAnalysis.scala | 18 ++++- .../dfhdl/compiler/stages/NamedAliases.scala | 58 ++++++++++++++++- .../scala/StagesSpec/NamedSelectionSpec.scala | 65 +++++++++++++++++++ .../dfhdl/compiler/patching/memberOps.scala | 2 + core/src/main/scala/dfhdl/hw/annotation.scala | 15 +++-- .../dfhdl/tools/toolsCore/Verilator.scala | 12 +++- 8 files changed, 185 insertions(+), 12 deletions(-) diff --git a/.claude/commands/new-stage.md b/.claude/commands/new-stage.md index 1cabc56c0..b9a3059ac 100644 --- a/.claude/commands/new-stage.md +++ b/.claude/commands/new-stage.md @@ -76,6 +76,13 @@ reconstructs it: construct itself. Safe. - A tag marking *why a stage synthesized a member* has no printed form and nothing regenerates it. **Unsafe** — and the failure is silent, because the IR path keeps working. +- Information a stage derives for a downstream TOOL integration (a lint waiver, e.g.) travels + as a printed **HW annotation** on the member (`member.addAnnotation(...)` from + `patching/memberOps.scala`), never as a tag: `csDFMember` prints every member's annotations, + and elaboration captures them back off the printed `@hw.annotation...` line, so the printout + stays the full contract. The model is `NamedAliases` annotating the unread bits of a value it + names with `Unused.Quiet(hi, lo)`, which the verilator config printer turns into a + bit-precise UNUSEDSIGNAL waiver (a compiler-minted name must not mint new lint noise). The trap is that a tag can be perfectly fix-point-safe (re-tagging is a no-op) and still break this. Idempotency and printability are independent requirements. diff --git a/compiler/ir/src/main/scala/dfhdl/compiler/ir/annotation.scala b/compiler/ir/src/main/scala/dfhdl/compiler/ir/annotation.scala index 7bef010a7..da22bc3c5 100644 --- a/compiler/ir/src/main/scala/dfhdl/compiler/ir/annotation.scala +++ b/compiler/ir/src/main/scala/dfhdl/compiler/ir/annotation.scala @@ -16,15 +16,31 @@ object annotation: ) enum Unused extends HWAnnotation derives ReadWriter: - case Quiet, Keep, Prune + /** `Quiet` optionally carries a bit range (`bitIdxHigh` downto `bitIdxLow`), narrowing the + * suppression to just those bits. The compiler mints such annotations itself when it names an + * anonymous value whose readers select only some of its bits (see `NamedAliases`). + */ + case Quiet(bitIdxHigh: ConfigN[Int] = None, bitIdxLow: ConfigN[Int] = None) + case Keep, Prune protected def `prot_=~`(that: HWAnnotation)(using MemberGetSet): Boolean = this == that lazy val getRefs: List[DFRef.TwoWayAny] = Nil def copyWithNewRefs(using RefGen): this.type = this + + /** The bit range this annotation narrows the unused-suppression to, when it has one. */ + def bitRangeOpt: Option[(Int, Int)] = this match + case Quiet(bitIdxHigh, bitIdxLow) => + (bitIdxHigh.toOption, bitIdxLow.toOption) match + case (Some(hi), Some(lo)) => Some((hi, lo)) + case _ => None + case _ => None def codeString(using Printer): String = this match - case Quiet => "@hw.annotation.unused.quiet" + case _: Quiet => + val rangeCS = bitRangeOpt.map((hi, lo) => s"($hi, $lo)").getOrElse("") + s"@hw.annotation.unused.quiet$rangeCS" case Keep => "@hw.annotation.unused.keep" case Prune => "@hw.annotation.unused.prune" + end Unused /** Purity marking. Elaboration is pure by default; `Pure(false)` marks it impure (its results * must not be cached), `Pure(true)` is the user's explicit trust override for the compiler's diff --git a/compiler/stages/src/main/scala/dfhdl/compiler/analysis/DBAnalysis.scala b/compiler/stages/src/main/scala/dfhdl/compiler/analysis/DBAnalysis.scala index 1efc1c126..d6452fe87 100644 --- a/compiler/stages/src/main/scala/dfhdl/compiler/analysis/DBAnalysis.scala +++ b/compiler/stages/src/main/scala/dfhdl/compiler/analysis/DBAnalysis.scala @@ -6,13 +6,29 @@ extension (designDB: DB) import designDB.getSet designDB.members.flatMap: case dfVal: DFVal if !dfVal.isAnonymous => + // a bit-ranged `Quiet` suppresses only its bits (see `getUnusedBitsAnnotValues`), + // so it does not put the value on the whole-value suppression list val isUnused = dfVal.meta.annotations.exists { - case u: annotation.Unused => true + case u: annotation.Unused => u.bitRangeOpt.isEmpty case _ => false } if (isUnused) Some(dfVal) else None case _ => None + // Named values carrying bit-ranged `Unused.Quiet` annotations, with their ranges in + // descending order. The compiler mints these in `NamedAliases` for the unread bits of a + // value it names, and a user may hand-write them; tools turn them into bit-precise waivers. + def getUnusedBitsAnnotValues: List[(DFVal, List[(Int, Int)])] = + import designDB.getSet + designDB.members.flatMap: + case dfVal: DFVal if !dfVal.isAnonymous => + val ranges = dfVal.meta.annotations.flatMap { + case u: annotation.Unused => u.bitRangeOpt + case _ => None + } + if (ranges.nonEmpty) Some((dfVal, ranges.sortBy((hi, _) => -hi))) + else None + case _ => None // TODO: need to apply a more stable tag when converting from mutable to immutable def getUnusedParamAnnotValues: List[DFVal] = import designDB.getSet diff --git a/compiler/stages/src/main/scala/dfhdl/compiler/stages/NamedAliases.scala b/compiler/stages/src/main/scala/dfhdl/compiler/stages/NamedAliases.scala index bd0e39f8a..672cf3786 100644 --- a/compiler/stages/src/main/scala/dfhdl/compiler/stages/NamedAliases.scala +++ b/compiler/stages/src/main/scala/dfhdl/compiler/stages/NamedAliases.scala @@ -50,6 +50,54 @@ private abstract class NamedAliases extends HierarchyStage: ch :: ch.getCBList.flatMap(cb => cb :: cb.members(MemberView.Flattened)) case relVal => List(relVal) } + // A compiler-minted name must not mint new lint noise. The value being named was anonymous, + // printed inline in the HDL, so no tool could warn about bits of it that nothing reads; once + // named it becomes a declared signal, and a linter (verilator's UNUSEDSIGNAL, e.g.) reports + // its unread bits, a warning the user's code has no handle on. So when the named value is a + // packed scalar whose every reader is a static bit selection, the bits no reader selects are + // returned as ranges (absolute indexes, descending), to be annotated `unused.quiet(hi, lo)` + // on the named member, which tool integrations turn into bit-precise waivers. An annotation, + // unlike a tag, is printed, so a printed stage output re-elaborates to the same waivers. + // Any reader that is not a static in-bounds bit selection conservatively uses every bit. + private def unusedBitRanges(group: List[DFVal])(using MemberGetSet): List[(Int, Int)] = + val dfType = group.head.dfType + val lowOpt: Option[Int] = dfType match + case b: DFBitsWL => b.lowIdxRef.getIntOpt + case DFUInt(_) | DFSInt(_) => Some(0) + case _ => None + (lowOpt, group.head.widthIntOpt) match + case (Some(low), Some(width)) => + val used = new Array[Boolean](width) + val fullUse = group.exists(_.getReadDeps.exists { + case sel: DFVal.Alias.ApplyRange => + (sel.idxHighRef.getIntOpt, sel.idxLowRef.getIntOpt) match + case (Some(hi), Some(lo)) if lo >= low && hi < low + width => + (lo to hi).foreach(i => used(i - low) = true) + false + case _ => true + case sel: DFVal.Alias.ApplyIdx => + sel.relIdx.get match + case DFVal.Alias.ApplyIdx.ConstIdx(i) if i >= low && i < low + width => + used(i - low) = true + false + case _ => true + case _ => true + }) + if (fullUse) Nil + else + val ranges = collection.mutable.ListBuffer.empty[(Int, Int)] + var i = width - 1 + while (i >= 0) + if (!used(i)) + val hi = i + while (i >= 0 && !used(i)) i -= 1 + ranges += ((hi + low, i + 1 + low)) + else i -= 1 + ranges.toList + case _ => Nil + end match + end unusedBitRanges + // One naming pass. Returns an empty list once nothing anonymous meets the criteria any more, // which is what terminates the loop in `transformSubDB`. private def collectPatches(db: DB)(using MemberGetSet, CompilerOptions): List[(DFMember, Patch)] = @@ -78,9 +126,13 @@ private abstract class NamedAliases extends HierarchyStage: .map(_.unzip) // for each group use just the head to create the named member, along with the members that // have to travel with it when its position cannot hold a name - .collect { case (firstAlias :: restOfAliases, suggestedName :: _) => - // we force set the underlying original name before it was anonymized - val namedMember = firstAlias.setName(suggestedName) + .collect { case (aliases @ (firstAlias :: restOfAliases), suggestedName :: _) => + // we force set the underlying original name before it was anonymized, and annotate + // the bits none of the group's readers select as quietly unused (`foldRight`, since + // `addAnnotation` prepends, keeps the annotations in descending range order) + val namedMember = unusedBitRanges(aliases).foldRight(firstAlias.setName(suggestedName)) { + (range, member) => member.addAnnotation(annotation.Unused.Quiet(range._1, range._2)) + } val moved = hoistAnchorOf(firstAlias).map(anchor => (anchor, hoistMembers(firstAlias))) (firstAlias, namedMember, restOfAliases, moved) }.toList diff --git a/compiler/stages/src/test/scala/StagesSpec/NamedSelectionSpec.scala b/compiler/stages/src/test/scala/StagesSpec/NamedSelectionSpec.scala index 220d7f7cd..b261f5ef9 100644 --- a/compiler/stages/src/test/scala/StagesSpec/NamedSelectionSpec.scala +++ b/compiler/stages/src/test/scala/StagesSpec/NamedSelectionSpec.scala @@ -67,6 +67,7 @@ class NamedSelectionSpec extends StageSpec(stageCreatesUnrefAnons = true): | val y_part = (x + d"16'1").bits | y := y_part(7, 0) | y_part(15, 8) | else + | @hw.annotation.unused.quiet(7, 0) | val y_part = (x + d"16'1").bits | y := y_part(15, 8) | val y_part = (x + d"16'2").bits @@ -92,6 +93,7 @@ class NamedSelectionSpec extends StageSpec(stageCreatesUnrefAnons = true): |class ID extends DFDesign: | val x = Wrapper <> IN | val y = Bits(8) <> OUT + | @hw.annotation.unused.quiet(15, 8) | val y_part = x.actual(0) | y := y_part(7, 0) |end ID @@ -188,7 +190,9 @@ class NamedSelectionSpec extends StageSpec(stageCreatesUnrefAnons = true): | val c = Bit <> IN | val x = SInt(9) <> IN | val y = UInt(8) <> OUT + | @hw.annotation.unused.quiet(8, 8) | val anon = (~x.bits).uint + | @hw.annotation.unused.quiet(8, 8) | val anon = x.bits.uint | y <> (( | if (c) anon(7, 0) @@ -217,9 +221,11 @@ class NamedSelectionSpec extends StageSpec(stageCreatesUnrefAnons = true): | process(all): | y := (( | if (c) + | @hw.annotation.unused.quiet(8, 8) | val anon = (~x.bits).uint | anon(7, 0) | else + | @hw.annotation.unused.quiet(8, 8) | val anon = x.bits.uint | anon(7, 0) | ): UInt[8] <> VAL) @@ -252,6 +258,7 @@ class NamedSelectionSpec extends StageSpec(stageCreatesUnrefAnons = true): | val anon_part: UInt[9] <> VAL = | if (d) p | else q + | @hw.annotation.unused.quiet(8, 8) | val anon = anon_part + d"9'1" | y <> (( | if (c) p(7, 0) @@ -332,6 +339,7 @@ class NamedSelectionSpec extends StageSpec(stageCreatesUnrefAnons = true): """|class ID extends RTDesign: | val u = UInt(20) <> IN | val o = UInt(20) <> OUT + | @hw.annotation.unused.quiet(0, 0) | val s_part = u.signed | val s = s_part(20, 1) | o <> s @@ -370,8 +378,11 @@ class NamedSelectionSpec extends StageSpec(stageCreatesUnrefAnons = true): | val o5 = Bits(16) <> OUT | val o1_part = a.uint + d"20'1" | o1 <> o1_part(19, 0) + | @hw.annotation.unused.quiet(19, 16) | val o2_part = a.uint | o2 <> o2_part(15, 0) + | @hw.annotation.unused.quiet(9, 4) + | @hw.annotation.unused.quiet(2, 0) | val o3_part = a(9, 0) | a(19, 10) | o3 <> o3_part(3) | o4 <> (a.uint + d"20'1").bits(19, 0) @@ -394,6 +405,7 @@ class NamedSelectionSpec extends StageSpec(stageCreatesUnrefAnons = true): """|class ID extends RTDesign: | val a = Bits(20) <> IN | val o = Bits(8) <> OUT + | @hw.annotation.unused.quiet(9, 8) | val s_part = a(9, 0) | a(19, 10) | val s = s_part(7, 0) | o <> s @@ -401,4 +413,57 @@ class NamedSelectionSpec extends StageSpec(stageCreatesUnrefAnons = true): |""".stripMargin ) } + // A compiler-minted name must not mint new lint noise: the bits of the named value that no + // reader selects are annotated as quietly unused, one annotation per contiguous range in + // descending order, which tool integrations (e.g. verilator's lint config) turn into + // bit-precise waivers. + test("Named selection annotates the unselected bits as quietly unused") { + class ID extends RTDesign: + val x = UInt(8) <> IN + val y = Bits(4) <> OUT + val z = Bits(2) <> OUT + y := (x + 1).bits(3, 0) + z := (x + 2).bits(4, 3) + + val id = (new ID).verilogNamedSelection + assertCodeString( + id, + """|class ID extends RTDesign: + | val x = UInt(8) <> IN + | val y = Bits(4) <> OUT + | val z = Bits(2) <> OUT + | @hw.annotation.unused.quiet(7, 4) + | val y_part = (x + d"8'1").bits + | y := y_part(3, 0) + | @hw.annotation.unused.quiet(7, 5) + | @hw.annotation.unused.quiet(2, 0) + | val z_part = (x + d"8'2").bits + | z := z_part(4, 3) + |end ID + |""".stripMargin + ) + } + // The annotation is printed DFHDL syntax (unlike a tag), so a printed stage output + // re-elaborates to the same IR and a re-run leaves it unchanged (print-safe fix-point). + test("A hand-written quietly-unused bit range is preserved by the stage") { + class ID extends RTDesign: + val x = UInt(8) <> IN + val y = Bits(4) <> OUT + @hw.annotation.unused.quiet(7, 4) + val y_part = (x + 1).bits + y := y_part(3, 0) + + val id = (new ID).verilogNamedSelection + assertCodeString( + id, + """|class ID extends RTDesign: + | val x = UInt(8) <> IN + | val y = Bits(4) <> OUT + | @hw.annotation.unused.quiet(7, 4) + | val y_part = (x + d"8'1").bits + | y := y_part(3, 0) + |end ID + |""".stripMargin + ) + } end NamedSelectionSpec diff --git a/core/src/main/scala/dfhdl/compiler/patching/memberOps.scala b/core/src/main/scala/dfhdl/compiler/patching/memberOps.scala index 10fd87460..ebcdec929 100644 --- a/core/src/main/scala/dfhdl/compiler/patching/memberOps.scala +++ b/core/src/main/scala/dfhdl/compiler/patching/memberOps.scala @@ -8,6 +8,8 @@ extension [T <: DFMember](member: T) getSet.set(member)(_.setMeta(_.setName(name))) def anonymize(using MemberGetSet): T = getSet.set(member)(_.setMeta(_.anonymize)) + def addAnnotation(annot: annotation.HWAnnotation)(using MemberGetSet): T = + getSet.set(member)(_.setMeta(_.addAnnotation(annot))) def removeTagOf[CT <: DFTag: ClassTag](using MemberGetSet): T = getSet.set(member)(_.setTags(_.removeTagOf[CT])) def tag[CT <: DFTag: ClassTag](customTag: CT)(using MemberGetSet): T = diff --git a/core/src/main/scala/dfhdl/hw/annotation.scala b/core/src/main/scala/dfhdl/hw/annotation.scala index 8e69bc187..75b74ba52 100644 --- a/core/src/main/scala/dfhdl/hw/annotation.scala +++ b/core/src/main/scala/dfhdl/hw/annotation.scala @@ -31,11 +31,18 @@ package annotation: final class setName(val name: String) extends StaticAnnotation object unused: - /** `quiet` suppresses the unused warning for the tagged value. + /** `quiet` suppresses the unused warning for the tagged value. An optional bit range + * (`bitIdxHigh` downto `bitIdxLow`) narrows the suppression to just those bits. */ - final class quiet(val isActive: Boolean) extends HWAnnotation: - def this() = this(true) - val asIR: ir.annotation.Unused = ir.annotation.Unused.Quiet + final class quiet( + val isActive: Boolean, + val bitIdxHigh: ir.ConfigN[Int], + val bitIdxLow: ir.ConfigN[Int] + ) extends HWAnnotation: + def this() = this(true, None, None) + def this(isActive: Boolean) = this(isActive, None, None) + def this(bitIdxHigh: Int, bitIdxLow: Int) = this(true, bitIdxHigh, bitIdxLow) + val asIR: ir.annotation.Unused = ir.annotation.Unused.Quiet(bitIdxHigh, bitIdxLow) /** `keep` suppresses the unused warning, and also attempts to keep the tagged value. */ diff --git a/lib/src/main/scala/dfhdl/tools/toolsCore/Verilator.scala b/lib/src/main/scala/dfhdl/tools/toolsCore/Verilator.scala index 6e93aa297..fc620e8b0 100644 --- a/lib/src/main/scala/dfhdl/tools/toolsCore/Verilator.scala +++ b/lib/src/main/scala/dfhdl/tools/toolsCore/Verilator.scala @@ -380,11 +380,19 @@ class VerilatorConfigPrinter(verilatorVersion: String, isToolInWindows: Boolean) matchWild = s"*: '${dfVal.getName}'*" ) .distinct.mkString("\n") + // Two sources: the truncation-assignment IR shape, and bit-ranged `@unused.quiet(hi, lo)` + // annotations (minted by `NamedAliases` for the unread bits of a value the compiler named, + // or hand-written). Verilator reports ALL of a signal's unused ranges in ONE message, + // comma-joined in descending order (`'x'[7:5,2:0]`, a single bit as `[3]`), so a signal's + // ranges must be joined the same way for the waiver to match. def lintOffUnusedBits: String = - designDB.getUnusedBitsValues.map: (dfVal, idxHigh, idxLow) => - val bitSel = + val fromNets = designDB.getUnusedBitsValues.map: (dfVal, idxHigh, idxLow) => + (dfVal, List((idxHigh, idxLow))) + (fromNets ++ designDB.getUnusedBitsAnnotValues).map: (dfVal, ranges) => + val bitSel = ranges.map: (idxHigh, idxLow) => if (idxHigh == idxLow) s"$idxHigh" else s"$idxHigh:$idxLow" + .mkString(",") lintOffCommand( rule = "UNUSEDSIGNAL", file = dfVal.fileNameFilter, From a8fe76a2ee9575dbc16f93f94f59aef69a7e5772 Mon Sep 17 00:00:00 2001 From: Oron Date: Tue, 25 Aug 2026 01:53:37 +0300 Subject: [PATCH 3/6] lib: the v95 IS_NEG macro yields a true 1-bit value, keeping width-aware linters quiet Reading the sign bit by shift leaves a width-wide result whose consumers in the signed ordering macros (`!`, `&&`) are 1-bit logical operators, so verilator's WIDTHTRUNC flagged every use site (32 warnings on util.SignedCmpSim, an error under --Werror-tool). The shift leaves only the sign bit, so a reduction-OR equals it exactly while producing a genuine 1-bit value, with no 32-bit constant mixing to trade in a WIDTHEXPAND instead. Conformance verified by util.SignedCmpSim under both verilator and iverilog on v95. Co-Authored-By: Claude Fable 5 --- compiler/stages/src/main/resources/dfhdl_defs.vh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/stages/src/main/resources/dfhdl_defs.vh b/compiler/stages/src/main/resources/dfhdl_defs.vh index fa7b09359..920c5b095 100644 --- a/compiler/stages/src/main/resources/dfhdl_defs.vh +++ b/compiler/stages/src/main/resources/dfhdl_defs.vh @@ -75,8 +75,11 @@ // Signed ordering over two vectors of the SAME width: the sign bits decide when they differ, // and an unsigned comparison of the magnitudes when they agree. The sign bit is read by SHIFT // rather than by bit-select, so an operand may be any expression (a bit-select would require an -// indexable primary, which a widened or arithmetic operand is not). -`define IS_NEG(a, width) (((a) >> ((width)-1))) +// indexable primary, which a widened or arithmetic operand is not). The shift result is as wide +// as the operand while its consumers (`!`, `&&`) are 1-bit logical operators, so it is narrowed +// by reduction-OR: the shift left only the sign bit, so the reduction equals it, and a genuine +// 1-bit value keeps width-aware linters (verilator's WIDTHTRUNC) quiet. +`define IS_NEG(a, width) ((|((a) >> ((width)-1)))) `define SIGNED_GREATER_THAN(a, b, width) \ ((`IS_NEG(a, width) && !`IS_NEG(b, width)) ? 1'b0 : /* a is negative, b is positive */ \ (!`IS_NEG(a, width) && `IS_NEG(b, width)) ? 1'b1 : /* a is positive, b is negative */ \ From a33ab994c498e931fe93523e3f90c0d823cf74d5 Mon Sep 17 00:00:00 2001 From: Oron Date: Tue, 25 Aug 2026 02:43:51 +0300 Subject: [PATCH 4/6] core+lib: Werror routes warnings through the error channel; elaboration checks pin relative positions With `Werror` the warnings ARE errors: they now travel the error channel with their full content (trapped as an exception or printed per `OnError`), instead of being unconditionally pre-printed to stderr with only a generic error taking their place. ElaborationChecksSpec sets `WError = true` file-wide, so its warning-producing designs assert their warnings' full content like any other error and nothing leaks to the console (the given must be file-scoped: the plugin-generated `__dfc` cannot capture a method-local given). `assertElaborationErrors` now rewrites same-file positions in the obtained message to offsets from the assertion's munit.Location anchor (the line the call's last argument list closes on): `L-9:17` reads "nine lines above". Expected strings are authored in the same relative form, so they survive line churn anywhere outside their own test (previously a single inserted line broke every expectation below it), and a failure's diff shows the correct form to paste. All 68 expectations migrated; the one manual try/catch test routes through the same relativization. Co-Authored-By: Claude Fable 5 --- .claude/commands/bugfix.md | 41 ++-- CLAUDE.md | 2 +- core/src/main/scala/dfhdl/core/Design.scala | 33 ++-- lib/src/test/scala/DesignSpec.scala | 35 +++- .../test/scala/ElaborationChecksSpec.scala | 181 ++++++++++-------- 5 files changed, 175 insertions(+), 117 deletions(-) diff --git a/.claude/commands/bugfix.md b/.claude/commands/bugfix.md index 55e8874f4..10a1b069a 100644 --- a/.claude/commands/bugfix.md +++ b/.claude/commands/bugfix.md @@ -1160,25 +1160,28 @@ exercise is a question, not a to-do. ### Position-sensitive elaboration tests -`ElaborationChecksSpec` expectations embed `::` of the offending expression. -scalafmt reflows the test design (a braces-on-one-line block becomes multi-line), which silently -shifts those positions. Write the design in the already-normalized indented form so reformatting -does not move it, and re-check the positions after running scalafmt. - -This is the general reason **scalafmt belongs before the final full-suite run, not after it**: -formatting rewrites the very spec files the suite just exercised, so a run that precedes it has to -be repeated. Format once the narrow specs are green, revert the unrelated churn scalafmt always -produces, then run the suite. - -Any edit that changes the file's LINE COUNT shifts every expectation below it, so adding a test in -the middle breaks unrelated tests that were passing. Append new tests at the end of the file. When -a mid-file edit is unavoidable (rewriting an existing test), do not hand-patch the fallout: munit -prints each expected/obtained pair, so drive the rewrite off the run log — extract the -`-Position:`/`+Position:` pairs and apply them to the source in ONE simultaneous pass (a -sequential pass can rewrite a value that a later rule then matches). Two or three iterations -converge, since a test with several expected errors only reveals its next stale position after the -first is fixed. Do the substitution with a script that preserves the file's CRLF bytes, not -`sed -i`, which rewrites the whole file's line endings and produces phantom diffs. +`ElaborationChecksSpec` expectations embed `::` of the offending expression, in +**relative form**: `L-9:17` reads "nine lines above this assertion's anchor". The anchor is what +`munit.Location` reports for the call, which is the line the call's LAST argument list CLOSES on +(not the line it opens on); `DesignSpec.relativizeLines` rewrites the obtained message to the same +form before diffing. Consequently: + +- Edits anywhere else in the file (adding a test mid-file included) no longer shift another + test's expectations. Only edits *inside* a test, between its design and its assertion's closing + paren, move that test's own offsets — scalafmt reflowing the test design is the usual cause, so + write designs in the already-normalized indented form. +- A stale offset is fixed by copying from the failure diff, which prints both sides in relative + form. No run-log-driven mass rewrites are needed any more. +- scalafmt still belongs **before** the final full-suite run, not after it: formatting rewrites + the very spec files the suite just exercised, so a run that precedes it has to be repeated. + +The spec also sets `given options.ElaborationOptions.WError = true` file-wide, so a test whose +design produces elaboration *warnings* must assert them: they arrive appended to the trapped +error message (full content, position included), followed by the +`Warnings found with -Werror enabled...` line — a warning can never silently leak to the console +from this spec. Note the givens must stay at FILE scope: a test-body-local (or local +`object Test` member) given cannot be captured by the plugin-generated `__dfc` +(`Could not find proxy for lazy var` / `failure to construct path` at compile time). --- diff --git a/CLAUDE.md b/CLAUDE.md index c14f529ab..b29a7997f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -104,7 +104,7 @@ Plugin options are `-P:dfhdl.plugin: