Skip to content

[Sim][SimToSV] Supplementing the infrastructure for Sim dialects#10146

Closed
nanjo712 wants to merge 1 commit into
llvm:mainfrom
nanjo712:feat/sim-infra
Closed

[Sim][SimToSV] Supplementing the infrastructure for Sim dialects#10146
nanjo712 wants to merge 1 commit into
llvm:mainfrom
nanjo712:feat/sim-infra

Conversation

@nanjo712
Copy link
Copy Markdown
Contributor

@nanjo712 nanjo712 commented Apr 7, 2026

This PR is part of #10131 , split from #10140.

It focuses on the introduce some change in Sim Dialect in order to migrate firrtl.printf / firrtl.fprintf / firrtl.fflush .

ClockType:$clock,
I1:$condition
I1:$condition,
DefaultValuedAttr<BoolAttr, "false">:$usePrintfCond
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to carry FIRRTL thing here. Could you try if it's possible to push PRINTF_COND to condition?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PrintCond represents a macro on the SV backend. If the usePrintCond attribute is not included in this operation, perhaps adding a sim.env_flag in Sim Dialect would be appropriate?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to create sv::MacroRefExprOp::create(rewriter, op->getLoc(), rewriter.getI1Type(), "PRINTF_COND_"); in LowerToHW, and assign PRINT_COND & condition to the condition in LowerToHW.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Treating printCond as an environment input, it degrades to a macro-based printing switch on the SV backend, while on non-SV backends, such as Arcilator, printing is toggled using command-line switches or environment variables. Would it be acceptable to introduce a sim.env_flag operation to retrieve this value?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

|I've considered using sv::MacroRefExprOp directly in LowerToHW, but I'm concerned this might be prematurely leaking SystemVerilog-specific semantics into the structural IR.

As discussed in #10131, the overarching goal of this series of PRs is to decouple the LowerToHW phase from SV-specific operations. My aim is to ensure that the IR remains backend-agnostic as long as possible. Consumers like Arcilator should not be forced to implement a Verilog preprocessor or handle macro expressions.

By using a more abstract sim.env_flag (or equivalent), we maintain a clean separation of concerns: LowerToHW defines what is being gated, while the backend-specific pass (SimToSV) defines how that gate is implemented (e.g., as a macro).

If we want to avoid sv ops in LowerToHW, what's the preferred way in CIRCT to represent an abstract simulation flag?

Comment thread lib/Dialect/Sim/Transforms/ProceduralizeSim.cpp
FormatHexOp, FormatOctOp, FormatCharOp, FormatHierPathOp,
FormatScientificOp, FormatFloatOp, FormatGeneralOp>(op);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this part please ask someone else.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. Since this part is specific to the sim dialect's formatting infrastructure, I'll loop in @fzi-hielscher as he is the primary author of this component.

Comment thread lib/Conversion/SimToSV/SimToSV.cpp Outdated
Comment thread lib/Conversion/SimToSV/SimToSV.cpp Outdated
@nanjo712 nanjo712 force-pushed the feat/sim-infra branch 2 times, most recently from cfdf630 to 9dd9102 Compare April 8, 2026 10:02
@nanjo712
Copy link
Copy Markdown
Contributor Author

nanjo712 commented Apr 8, 2026

Hi @fzi-hielscher, I'm implementing the collection and string construction logic for sim.fmt ops. I'd appreciate your insights on whether this implementation aligns with your original design for the simulation dialect. Thanks!

@fzi-hielscher
Copy link
Copy Markdown
Contributor

Thanks a lot for your effort, @nanjo712. Your PR here touches on several longstanding pain points which are unfortunately to some extent interdependent and cross dialect boundaries. So, I hope you understand that I won't be able to give you many definite answers. Let me try to structure it a bit:

How should we handle stream/file resources in the Sim Dialect?

The general design of the GetFileOp looks good to me, but instead of an I32 it should return a dedicated !sim.output_stream type. The concrete implementation of a stream handle is a backend detail. I'd appreciate if you could move the addition of those to a dedicated PR. We will then likely also need special operations for stdout and stderr.

How do we get rid of the SV macros created by the FIRRTL frontend?

We discussed that during today's Open Developer Meeting and we all agreed that it would be great to get to a "clean" core dialect representation from FIRRTL. Unfortunately, the macros appear to be pretty integral to the Chisel to SV flow at the moment and no one had a straight-forward plan on how to deal with this. We might have to decide this on a case by case basis. For some macros we maybe could just add an option to the respective lowering pass, so it does not get emitted in the first place, if the user is targeting arcilator instead of SV.
@uenoku suggested the addition of a hw.choice operation, but there are apparently some complications regarding FIRRTL probes and XMRs.
I'm not opposed to have an operation providing runtime configuration like the sim.env_flag you suggested. Though, the closest SV equivalent to me here would be a $plusarg instead of a macro. This already exists in the sim dialect, however, I don't think I've ever used it, so I don't know how well it is supported right now.

How do we sequence side-effecting operations in non-procedural regions?

E.g., how do we specify that sim.fflush occurs after a sim.print if they happen on the same clock edge. There is a loose assumption that side-effects should be occurring in the order as their operations appear in the IR, but this is never actually enforced and to some extent clashes with the notion that in hardware things happen simultaneously (we could have lengthy "philosophical" discussions on whether the core dialects represent an AST or a netlist, but I don't think it would be particularly productive 😉).
I made a suggestion on how to deal with this a long time ago (#7676), but there are definitely other options. E.g., a token-based approach like in the MLIR async dialect. Or we could just require the frontend to put sequenced operations into the same procedural region and get rid of the non-procedural variants entirely.
One problem here is that the intent on the FIRRTL side is not that well specified, see #7973.

So, please forgive me for having to ask you to split this into even smaller chunks (and ideally incremental steps), which will inevitably result in more discussions. It would be a really great outcome if we could get this sorted out. 👍

@nanjo712
Copy link
Copy Markdown
Contributor Author

nanjo712 commented Apr 9, 2026

Thank you for submitting this issue to the ODM and for providing detailed feedback! It's encouraging to see that we've reached a consensus on the main pain points.

I now fully understand that there are indeed many unresolved issues involved, so breaking this PR down into smaller modules should allow us to focus on single design decisions and reduce review pressure.

I will close this large PR and start submitting these smaller modules, beginning with GetFileOp. Thank you again for your guidance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants