[SimToSV] Lower sim.triggered to sv.always

[nanjo712]

Lower sim.triggered to sv.always.

Assisted-by: Codex: GPT-5.4

[nanjo712]

Currently, it simply reduces the value of sim.triggered to sv.always without performing any merging operation. This will cause all sim.triggered events to execute in parallel.

I think it's not the form we want, since it is not aligned with the current lowering path in LowerToHW, so for the moment it's only a prototype. Let me know what you think! @fzi-hielscher 😊

[circt-bot]

Results of circt-tests run for a729db1 compared to results for 649fd85: no change to test results.

[fzi-hielscher]

Hi @nanjo712,

the lowering looks fine to me. But couldn't we also add a synthesis guard around the always and thereby solve #10489? I don't think we have to match the current LowerToHW path exactly, at least not for now.

Having a separate SV process for each TriggeredOp is likely going to cause a mess in practice. However, it is technically correct, as we have decided to not establish an order between the operations. My suggestion to fix this would be to let ProceduralizeSim squash all sim.triggered operations per clock and module into a single one, so we will have a deterministic order after that point. That won't be the cleanest of solutions, but it should be good enough until the semantics on the FIRRTL side have been figured out.

[nanjo712]

Sorry, I'm a little confused, and I hope you can help me clarify this part of the logic.

For #10489, I think adding a synthesis guard around the always generated when lowering sim.triggered would indeed partially address the issue. However, it would not fully fix it as long as ProceduralizeSim may still emit hw.triggered, since the original issue was observed on the hw.triggered-based path.

For ProceduralizeSim, in #10422 you mentioned:

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.

Did you mean squashing all sim.triggered operations of an HWModule into a common sim.triggered instead? Sorry if I misunderstood, but if ProceduralizeSim continues to emit hw.triggered, then I think #10489 would still be somewhat difficult to fix cleanly, since it is not obvious where to add the synthesis guard.

If we instead update ProceduralizeSim to squash multiple sim.triggered operations into a common sim.triggered, I am also not sure how we should combine triggers with the same clock but different enable conditions. Would the intended representation be something like this?

sim.triggered %clock {
  scf.if %cond1 {
    // operations from the first sim.triggered
  }
  scf.if %cond2 {
    // operations from the second sim.triggered
  }
}

That is, the common sim.triggered would represent the shared clock trigger, while the original per-trigger enable conditions would be lowered into conditional regions inside the body.

[circt-bot]

Results of circt-tests run for aa1c16b compared to results for a729db1: no change to test results.

[fzi-hielscher]

Did you mean squashing all sim.triggered operations of an HWModule into a common sim.triggered instead? Sorry if I misunderstood, but if ProceduralizeSim continues to emit hw.triggered, then I think #10489 would still be somewhat difficult to fix cleanly, since it is not obvious where to add the synthesis guard.

Sorry for the confusion. Since you added this lowering, I assumed that you decided against converting sim.triggered to hw.triggered, which also sounds like a reasonable choice.
How about this: We add an option to ProceduralizeSim that controls whether it squashes into sim.triggered or hw.triggered. For SV lowering we use sim.triggered, which will give us the synthesis guards. For Arcilator lowering we use hw.triggered, which can then trivially be lowered to arc.execute.

That is, the common sim.triggered would represent the shared clock trigger, while the original per-trigger enable conditions would be lowered into conditional regions inside the body.

Exactly.

[nanjo712]

How about this: We add an option to ProceduralizeSim that controls whether it squashes into sim.triggered or hw.triggered. For SV lowering we use sim.triggered, which will give us the synthesis guards. For Arcilator lowering we use hw.triggered, which can then trivially be lowered to arc.execute.

That sounds really nice! Let's do it in the next PR.

For this PR, let me add a synthesis guard for sv.always first.

Thanks a lot, I really appreciate your feedback!

[circt-bot]

Results of circt-tests run for 1b60bd6 compared to results for aa1c16b: no change to test results.

[circt-bot]

Results of circt-tests run for 85acb84 compared to results for 1b60bd6: no change to test results.

[fzi-hielscher]

👍

[circt-bot]

Results of circt-tests run for 63a14bf compared to results for 85acb84: no change to test results.