[Sim] Extract common logic of ProceduralizeSim for reuse

[nanjo712]

Assisted-by: Codex: GPT-5.4

Extract common logic of ProceduralizeSim for reuse.

[nanjo712]

I think it's ready for review now! 🚀

[fzi-hielscher]

Thanks, @nanjo712. My impression is that we should not be embedding the ProceduralizeSim logic into other passes and instead should begin phasing out both ProceduralizeSim and the non-procedural versions of the sim operations.
We've seen the issues regarding side-effects with GetFileOp and it is likely only going to get worse from here.

My suggestion would be to add a sim.triggered operation, that essentially works the same way as hw.triggered, but make it not IsolatedFromAbove and add an (optional) condition operand to it. E.g.,:

  %msg = sim.fmt.literal "foo"
  sim.print %msg on %clk if %cond

could be substituted by

  %msg = sim.fmt.literal "foo"
  sim.triggered %clock if %cond {
    sim.proc.print %msg
  }

And then let the FIRRTL frontend directly build the sim.triggered op. I think this would simplify the lowering process. What's your impression?

I've also just merged the "SparseOpSCC" utility: #10305 This might come in handy in situations where we need to manipulate subgraphs of sim operations.

[nanjo712]

Thanks for feedback, @fzi-hielscher .The reason I'm implementing it this way is that FIRRTL itself doesn't contain a procedural print operation (only firrtl.print, similar to sim.print). Some proceduralization step is still needed to create a procedural container in the FIRRTL->Sim flow. Therefore, I've relaxed the input Ops allowed by ProceduralizeSim to directly convert from firrtl.print to sim.proc.print in #10441, thus bypassing the need for sim.print.

I think sim.triggered is a good suggestion; as a procedural container specifically for simulation operations, I believe it would be better than using hw.triggered. However, I have some questions about adding an optional conditional operand:

As I understand it, if there are two print operations with the same clock but different conditions, in such a design, this might be split into two different sim.triggered blocks, for example:

sim.print %msg1 on %clk if %cond1 // or some ops like firrtl.print
sim.print %msg2 on %clk if %cond2

This would become:

sim.triggered %clk if %cond1 {
  sim.proc.print %msg1
}
sim.triggered %clk if %cond2 {
  sim.proc.print %msg2
}

This seems to make the execution order of these two operations very ambiguous.

Would it be better to use sim.triggered as an event-scoped region per clock, and keep the individual print conditions inside the region? For example:

sim.triggered %clk {
  scf.if %cond1 {
    sim.proc.print %msg1
  }
  scf.if %cond2 {
    sim.proc.print %msg2
  }
}

[nanjo712]

I think I may still be missing part of the intended benefit of sim.triggered.

If the goal is to avoid the pattern of:

non-procedural side-effect op
-> later proceduralization / rematerialization / cleanup
-> procedural side-effect op

then I am not sure that introducing sim.triggered alone fundamentally changes the situation. The source FIRRTL operation is still a non-procedural firrtl.print: it carries a clock and an enable, and it is not already nested in a procedural region.

Because of that, it seems that some part of the FIRRTL-to-Sim lowering still has to perform a similar transformation from this non-procedural, event-qualified operation into a procedural/event-scoped representation. sim.triggered may be a better target container for that representation than hw.triggered, but I do not yet see how it avoids the need for this proceduralization step altogether.

Maybe the intended distinction is that FIRRTL lowering would build sim.triggered directly, and that sim.triggered would be non-IsolatedFromAbove so that we can avoid most of the rematerialization/capture/cleanup logic?

[fzi-hielscher]

Maybe the intended distinction is that FIRRTL lowering would build sim.triggered directly, and that sim.triggered would be non-IsolatedFromAbove so that we can avoid most of the rematerialization/capture/cleanup logic?

I'd like to keep the FIRRTL to Sim lowering as simple as possible. The primary benefit of lowering directly to sim.triggered would be that we can safely split side-effecting operations while guaranteeing correct order and condition on the separated side-effects. I.e., we do not need to defer the effect of a sim.get_file anymore. Instead we make it a procedural operation and let the FIRRTL frontend put it right before the user op of the file.

This seems to make the execution order of these two operations very ambiguous.

True. The order of side-effects between FIRRTL operations is a different a story. FIRRTL doesn't really provide a solid specification here. The question is whether we want to establish a defined order during FIRRTL lowering despite that and how much effort we put into it. I cannot provide an authoritative answer for that question. But my suggestion would be to keep it simple and aligned with the current SV lowering, which does not establish an order. We can discuss if and how we should refine this later. By running ProceduralizeSim shortly after LowerToHW, and letting it squash all sim.triggered operations of a HWModule into a common hw.triggered op, we should generally get the desired behavior without having to explicitly specify it for now.

Whether sim.triggered should have a condition operand or not is also a fair question. I'm leaning towards yes, as it allows us to directly represent the current behavior of the non-procedural operations without having to introduce the SCF dialect. If that should turn out to be a poor decision, it will be pretty easy to remove it again.

I tried a "big bang" solution to all of this a year and a half ago. It didn't work out too well. 😉
So, my recommendation now is to do small targeted steps towards an initial working pipeline for both arcilator and SV, and then see what can or needs to be improved.

[nanjo712]

This is indeed a complex issue. I agree with your point: let's start with some incremental changes, at least to avoid making the changes too drastic.

Let's begin by introducing a sim.triggered, which I've already done at #10450. We can discuss the details of how it's consumed by the backend later.