8000 [FIRRTL][SV] Add fflush operation by uenoku · Pull Request #8420 · llvm/circt · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[FIRRTL][SV] Add fflush operation #8420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Apr 18, 2025
Merged

[FIRRTL][SV] Add fflush operation #8420

merged 2 commits into from
Apr 18, 2025

Conversation

uenoku
Copy link
Member
@uenoku uenoku commented Apr 15, 2025 •
edited
Loading

Add support for fflush operation in FIRRTL and SV dialects. This operation maps to SystemVerilog's $fflush system task, which flushes buffered output to files.

The implementation adds FFlushOp to FIRRTL dialect with clock, condition operands and optional filename with substitutions, adds FFlushOp to SystemVerilog dialect with optional file descriptor, implements lowering from FIRRTL to SystemVerilog, guards fflush with #ifndef SYNTHESIS like other simulation constructs, and adds visitor pattern support in both dialects.

Similar to StopOp in implementation pattern and behavior.

circuit PrintTest:
  public module PrintTest :
    input clock : Clock
    input cond : UInt<1>
    input a : UInt<8>
    node c = UInt<8>(97)
    fflush(clock, cond)

    fflush(clock, cond, "{{SimulationTime}}%d.txt", a)
module PrintTest(
  input       clock,
              cond,
  input [7:0] a
);

  logic [31:0] ___circt_lib_logging3A3AFileDescriptor3A3Aget_0;
  `ifndef SYNTHESIS
    always @(posedge clock) begin
      if ((`PRINTF_COND_) & cond) begin
        $fflush(`PRINTF_FD_);
        ___circt_lib_logging3A3AFileDescriptor3A3Aget_0 = __circt_lib_logging::FileDescriptor::get($sformatf("%0t%d.txt",
                                                                                                             $time,
                                                                                                             a));
        $fflush(___circt_lib_logging3A3AFileDescriptor3A3Aget_0);
      end
    end // always @(posedge)
  `endif // not def SYNTHESIS
endmodule

uenoku
uenoku commented Apr 15, 2025
View reviewed changes
seldridge
seldridge reviewed Apr 15, 2025
View reviewed changes
Copy link
Member
@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

Only went through part of the way as I realized I'm reviewing while I think you're still coding. 🙃

lib/Conversion/FIRRTLToHW/LowerToHW.cpp Outdated
addToIfDefBlock("SYNTHESIS", std::function<void()>(), [&]() {
addToAlwaysBlock(clock, [&]() {
circuitState.usedPrintf = true;
circuitState.addFragment(theModule, "PRINTF_FD_FRAGMENT");
Copy link
Member

Choose a reason for hiding this comment

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

Is this unused here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed to flush default FD when file name was not provided, so it's used now.

@uenoku uenoku force-pushed the dev/hidetou/fflush branch from c5ccc80 to 4c2c853 Compare April 17, 2025 01:44
@uenoku uenoku changed the base branch from main to dev/hidetou/format-string April 17, 2025 01:45
@uenoku uenoku force-pushed the dev/hidetou/fflush branch 3 times, most recently from 755c420 to 4bb5380 Compare April 17, 2025 03:02
@uenoku uenoku marked this pull request as ready for review April 17, 2025 03:06
@uenoku uenoku requested a review from darthscsi as a code owner April 17, 2025 03:06
seldridge
seldridge approved these changes Apr 17, 2025
View reviewed changes
Copy link
Member
@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

LGTM

The fact that this also takes a format string is funky. However, it's consistent with fprintf.

Thank you for finishing the fine details on this, like keeping the FIREmitter up to date. 👍

Base automatically changed from dev/hidetou/format-string to main April 18, 2025 01:16
uenoku added 2 commits April 17, 2025 18:20
@uenoku
[FIRRTL][SV] Add fflush operation
be38d82 
Add support for fflush operation in FIRRTL and SystemVerilog dialects.
This operation maps to SystemVerilog's $fflush system task, which flushes
buffered output to files.

The implementation adds FFlushOp to FIRRTL dialect with clock and condition
operands, adds FFlushOp to SystemVerilog dialect with optional file descriptor,
implements lowering from FIRRTL to SystemVerilog, guards fflush with #ifndef
SYNTHESIS like other simulation constructs, and adds visitor pattern support
in both dialects.

Similar to StopOp in implementation pattern and behavior.
@uenoku
Address comments
16a5636
@uenoku uenoku force-pushed the dev/hidetou/fflush branch from 4bb5380 to 16a5636 Compare April 18, 2025 01:37
@uenoku uenoku merged commit 5ec0585 into main Apr 18, 2025
5 checks passed
@uenoku uenoku deleted the dev/hidetou/fflush branch April 18, 2025 02:06
KelvinChung2000 pushed a commit to KelvinChung2000/circt that referenced this pull request Apr 22, 2025
@uenoku @KelvinChung2000
[FIRRTL][SV] Add fflush operation (llvm#8420)
2c1eab6 
Add support for fflush operation in FIRRTL and SV dialects. This operation maps to SystemVerilog's $fflush system task, which flushes buffered output to files.

The implementation adds FFlushOp to FIRRTL dialect with clock, condition operands and optional filename with substitutions, adds FFlushOp to SystemVerilog dialect with optional file descriptor, implements lowering from FIRRTL to SystemVerilog, guards fflush with #ifndef SYNTHESIS like other simulation constructs, and adds visitor pattern support in both dialects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seldridge seldridge seldridge approved these changes

@darthscsi darthscsi Awaiting requested review from darthscsi darthscsi is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@uenoku @seldridge
0