Skip to content

[SimToSV] support lowering stdout/stderr#10283

Merged
fzi-hielscher merged 5 commits intollvm:mainfrom
nanjo712:feat/sim-to-sv-print-stdout-stderr
Apr 25, 2026
Merged

[SimToSV] support lowering stdout/stderr#10283
fzi-hielscher merged 5 commits intollvm:mainfrom
nanjo712:feat/sim-to-sv-print-stdout-stderr

Conversation

@nanjo712
Copy link
Copy Markdown
Contributor

Support lowering stdout/stderr in SimToSV.

Comment thread lib/Conversion/SimToSV/SimToSV.cpp Outdated
@fzi-hielscher
Copy link
Copy Markdown
Contributor

Thanks, @nanjo712.
I'm not sure if it is going to work out, but it would be nice if we could treat streams more like normal values here. That is, add a type conversion from sim::OutputStreamType to I32 and add operation conversion patterns for the two ops. We will have to insert an UnrealizedConversionCastOp when creating the FWriteOp. This should then simply fold away after conversion. It could become more of a problem when we add support for the GetFileOp.

@uenoku
Copy link
Copy Markdown
Member

uenoku commented Apr 22, 2026

One tricky point with FIRRTL's fwrite semantics is we can literally make the file name dynamic, so it would be necessary to get file descriptor in the same cycle/always block.

@nanjo712 nanjo712 force-pushed the feat/sim-to-sv-print-stdout-stderr branch from 8799fa7 to fe4949b Compare April 23, 2026 17:24
@nanjo712 nanjo712 force-pushed the feat/sim-to-sv-print-stdout-stderr branch from fe4949b to 4545d21 Compare April 23, 2026 17:46
Comment thread lib/Conversion/SimToSV/SimToSV.cpp Outdated
@nanjo712
Copy link
Copy Markdown
Contributor Author

Sorry for the delayed follow-up on this PR — I was caught up with some personal matters and wasn’t able to address the review comments in time.

I’ve now updated the implementation to reflect the feedback. I’d appreciate it if you could take another look.

Copy link
Copy Markdown
Contributor

@fzi-hielscher fzi-hielscher left a comment

Choose a reason for hiding this comment

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

Sorry for the delayed follow-up on this PR — I was caught up with some personal matters and wasn’t able to address the review comments in time.

I've added the print operations two years ago, I'm in no hurry. 😉

We should (hopefully) be able to resolve the conversion casts on-the-fly without needing materializations and reconciliation:
First, mark them as dynamically illegal if they carry any type we need to convert.

target.addDynamicallyLegalOp<:UnrealizedConversionCastOp>([&](UnrealizedConversionCastOp op){
      return typeConverter.isLegal(op);
 });

Then add a conversion pattern for them that checks if the converted input types match the converted result types. If so, we can directly replace the op with the values provided by the adaptor.

Comment thread lib/Conversion/SimToSV/SimToSV.cpp Outdated
Comment thread lib/Conversion/SimToSV/SimToSV.cpp Outdated
Comment thread lib/Conversion/SimToSV/SimToSV.cpp Outdated
@nanjo712
Copy link
Copy Markdown
Contributor Author

Thank you for your feedback. I think your suggestion is indeed a good one, and I have revised the code accordingly.

@nanjo712 nanjo712 force-pushed the feat/sim-to-sv-print-stdout-stderr branch from b86b3b2 to 36f8268 Compare April 25, 2026 09:04
@nanjo712 nanjo712 requested a review from fzi-hielscher April 25, 2026 09:05
Copy link
Copy Markdown
Contributor

@fzi-hielscher fzi-hielscher left a comment

Choose a reason for hiding this comment

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

Nice, thanks.

Comment thread test/Conversion/SimToSV/lower-print-formatted-proc-to-sv-errors.mlir Outdated
@fzi-hielscher fzi-hielscher merged commit 216dc3d into llvm:main Apr 25, 2026
6 checks passed
@nanjo712 nanjo712 deleted the feat/sim-to-sv-print-stdout-stderr branch April 25, 2026 11:43
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