Skip to content
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.

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

[Arc][Sim] Lower Sim DPI func to func.func and support dpi call in Arc #7386

Merged
merged 7 commits into from
Aug 7, 2024

Conversation

uenoku
Copy link
Member

@uenoku uenoku commented Jul 25, 2024

This PR implements initial support for lowering Sim DPI operations to Arc.

  • sim::LowerDPIFuncPass implements lowering from sim.dpi.func to func.func that respects C-level ABI.
  • acc::LowerStatePass is modified to allocate states and call functions for sim.dpi.call op.

In the follow-up I'll rename sim.dpi.call to sim.func.call since now sim.dpi.call accepts both sim.dpi.func and func.func.

@uenoku uenoku force-pushed the dev/hidetou/arc-dpi branch from df12340 to 928d398 Compare July 25, 2024 09:32
lib/Dialect/Arc/ArcOps.cpp Outdated Show resolved Hide resolved
@sequencer
Copy link
Contributor

I think in the future. We may require the ClockGen intrinsic for triggering clock and reset. This is the only Verilog simulation blackbox that t1 use in the Verilator/VCS. It should be lowered to both Verilog and ArcOp.

@uenoku uenoku force-pushed the dev/hidetou/arc-dpi branch from 928d398 to 3f09368 Compare July 25, 2024 09:42
@uenoku uenoku changed the title [Arc] Lower Sim DPI ops to Arc [Arc][Sim] Lower Sim DPI func to func.func and support dpi call in Arc Jul 31, 2024
@uenoku uenoku force-pushed the dev/hidetou/arc-dpi branch 4 times, most recently from b6a8560 to 718cfa2 Compare August 5, 2024 08:31
@uenoku uenoku marked this pull request as ready for review August 5, 2024 08:31
@uenoku uenoku force-pushed the dev/hidetou/arc-dpi branch from 718cfa2 to 2e949e2 Compare August 5, 2024 08:45
Copy link
Member

@maerhart maerhart left a comment

Choose a reason for hiding this comment

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

LGTM! Really cool that arcilator can deal with DPI calls now! 🎉

lib/Dialect/Sim/Transforms/LowerDPIFunc.cpp Show resolved Hide resolved
lib/Dialect/Sim/SimOps.cpp Outdated Show resolved Hide resolved
integration_test/arcilator/JIT/dpi.mlir Outdated Show resolved Hide resolved
lib/Dialect/Sim/Transforms/LowerDPIFunc.cpp Outdated Show resolved Hide resolved
lib/Dialect/Sim/Transforms/LowerDPIFunc.cpp Show resolved Hide resolved
lib/Dialect/Sim/Transforms/LowerDPIFunc.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

This is really really cool! Thanks for also adding an integration test with Arcilator's JIT to actually give this a practical check 🙇 🙌 🥳

hw.module @adder(in %clock : i1, in %a : i32, in %b : i32, out c : i32) {
%seq_clk = seq.to_clock %clock

%0 = sim.func.dpi.call @dpi(%a, %b) clock %seq_clk : (i32, i32) -> i32
Copy link
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity: since the sim.func.dpi.call function seems like it accepts both sim.func.dpi and func.func callables, would this code also work with a second sim.func.dpi.call that calls the @adder_func directly?

Copy link
Member Author

Choose a reason for hiding this comment

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

Directly calling @adder_func doesn't work since it's output value is passed by reference in argument. If @adder_func was a normal dataflow-ish func.func such as func.func @adder_func_ok(%arg0: i32, %arg1: i32) -> i32, we can call it directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooooh you're right, sorry, I overlooked that difference in the operand layout. Makes sense!

@uenoku uenoku merged commit 9828707 into main Aug 7, 2024
4 checks passed
@uenoku uenoku deleted the dev/hidetou/arc-dpi branch August 7, 2024 04:51
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.

4 participants