When’ Requirement: Merging ‘setWhen’ and ‘clearWhen’ in Verilog Output

[dreamflyings]

I wrote RTL code using SpinalHDL and generated Verilog. When I checked the output using Spyglass software, I encountered numerous warnings related to ‘multiple if statements assigning the same variable.’ Upon reviewing the code, I realized that these warnings were caused by the usage of setWhen and clearWhen. After discussing this, we believe that it’s better to use if/else if constructs whenever possible, as it improves code readability. What are your thoughts on this? Below is my demo code (where the AI_* case class were generated by AI prompts)

import spinal.core._
import spinal.lib._

case class AI_WhenBuilder() {
 private var stack: List[(Bool, () => Unit)] = Nil

 def when(cond: Bool)(body: => Unit): this.type = {
 stack = (cond, () => body) :: stack
 this
 }

 def elsewhen(cond: Bool)(body: => Unit): this.type = {
 stack = (cond, () => body) :: stack
 this
 }

 def apply(cond: Bool)(body: => Unit): this.type = {
 stack = (cond, () => body) :: stack
 this
 }

 def otherwise(body: => Unit): Unit = {
 val (lastCond, lastBody) = stack.head
 val ctx = spinal.core.when(lastCond)(lastBody())
 stack.tail.foreach { case (cond, bodyFunc) =>
 ctx.elsewhen(cond)(bodyFunc())
 }
 ctx.otherwise(body)
 }
}

class SetClearTest() extends Component {
 val start = in(Bool())
 val end = in(Bool())//.setName("end")
 val o = out(Bool())
 val locked = RegInit(False)
 when(start){
 o := start
 locked := True
 }.otherwise{
 o := locked
 when(end){
 locked := False
 }
 }

 val ctx = WhenBuilder()
 val ctx_locked = RegInit(False)
 ctx.when(end ){ctx_locked := False}
 ctx.when(start){ctx_locked := True}
 val oo = out(Bool())
 oo := start | ctx_locked

 val setClr_locked = RegInit(False)
 val ooo = out(Bool())
 setClr_locked.setWhen(start).clearWhen(end)
 ooo := start | setClr_locked

 val ai_ctx = AI_WhenBuilder()
 val ai_ctx_locked = RegInit(False)
 ai_ctx.when(start){ai_ctx_locked := True}
 ai_ctx.when(end ){ai_ctx_locked := False}
 ai_ctx.otherwise({})
 val aio = out(Bool())
 aio := start | ai_ctx_locked
 noIoPrefix()
}

object SetClearTest_gen extends App{
 SpinalConfig(targetDirectory = "./verilog/SetClearTest/").generateVerilog(new SetClearTest()).printPruned()
}

[andreasWallner]

I think there is a misunderstanding where this behavior comes from: it's not the current implementation of WhenBuilder, it's that setWhen and clearWhen don't (and can't) track the original builder.

You can easily test this by adding another variant:

  val test = RegInit(False)
  when(start) { test := True} elsewhen(end){ test := False}

Which will generate

      if(start) begin
        test <= 1'b1;
      end else begin
        if(end_1) begin
          test <= 1'b0;
        end
      end

so as long as you use when, elsewhen and otherwise you'll also get a matching structure in the generated code.

So why does this happen with setWhen and clearWhen? If you look at the implementation you'll see that both just generate a seperate when clause which is not linked:

def setWhen(cond: Bool)(implicit loc: Location): Bool   = { when(cond){ this := True }; this }

TBH I don't see a simple way of changing this in the frontend, simply because stuff like this is totally allowed:

val test = RegInit(False)
test.setWhen(a)
when(b) { test := bb }
test.clearWhen(c)

One could group those assignments in the backend, not rocket-science but also not trivial - and in general we try to do as few "optimizations" in the backend. Output that's not matching the SpinalHDL would be one of the worst case bugs - and less modifications means less that can go wrong.
(The other question would be which camp to make happy that way - those that want the verilog to mimic the structure of the SpinalHDL, or those that e.g.want one process per variable.)

PS: which scala version are you using, the example that you posted did not compile for me as-is for 2.12

[dreamflyings]

Thank you very much for your detailed reply.

Regarding this issue, opinions vary from person to person, and it mainly depends on the thoughts of the majority and the actual necessity.
Of course, I also hope that there could be fewer warning entries in Spyglass, but this is not a must or an urgent need. These kinds of things can be done gradually; it is most important to first do what is important and urgent.

The code I provided can be compiled and generate Verilog in my environment. In environments 2.11.12, 2.12.4, and 2.12.18, the aforementioned code can run (since it is AI-generated code (quite cheeky, I will eventually delegate some authority to AI, so might as well start sooner rather than later)), it does have some compatibility issues, IntelliJ will show red bars, but it does not affect the compilation and generation. I am using our project without any special settings, and I am also very curious about this issue!

[andreasWallner]

Damn me, I trusted IntelliJ - not the first "good code red" that I've seen...