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

[Moore][Arc][LLHD] Moore to LLVM lowering issues #8012

Open
likeamahoney opened this issue Dec 20, 2024 · 4 comments
Open

[Moore][Arc][LLHD] Moore to LLVM lowering issues #8012

likeamahoney opened this issue Dec 20, 2024 · 4 comments

Comments

@likeamahoney
Copy link

Hi all!

I am trying to simulate a SystemVerilog code (listed below) using arcilator:

module dff(D, clk, Q);
    input D; // Data input 
    input clk; // clock input 
    output reg Q; // output Q

    // Simple DFF
    always @(negedge clk) 
    begin
        Q <= D; 
    end 
endmodule

with such pipeline:

circt-verilog dff.sv | arcilator

arcilator fails with error: 'llhd.process' op has regions; not supported by ConvertToArcs

Then I try to use a different pipeline with circt-opt :

circt-verilog dff.sv | circt-opt --llhd-early-code-motion --llhd-temporal-code-motion --llhd-desequentialize --llhd-sig2reg -canonicalize | arcilator

acilator fails with another error: failed to legalize operation 'seq.clock_inv'

What is necessary to be able to run arcilator on such simple DFF example?

@likeamahoney likeamahoney changed the title [Moore][Arc][LLHD] Moore to LLVM lowering issues [bug][Moore][Arc][LLHD] Moore to LLVM lowering issues Dec 20, 2024
@likeamahoney likeamahoney changed the title [bug][Moore][Arc][LLHD] Moore to LLVM lowering issues [Moore][Arc][LLHD] Moore to LLVM lowering issues Dec 20, 2024
@maerhart
Copy link
Member

Hi @likeamahoney

Thanks for checking out SystemVerilog simulation with Arcilator!

This lowering path is still in early development and the issue you are encountering is because of using negedge while we currently only support posedge upstream (because that's what Chisel/FIRRTL uses). The fix is simple: just add a lowering pattern for seq.clock_inv to LowerArcToLLVM.cpp.

I have a branch with some fixes and additional features (including that one) that should be enough to fully lower your example, if you want to give it a try. I hope to upstream that sometime soon. On the branch, you can simply run arcilator directly after circt-verilog because the LLHD pipeline was added to circt-verilog.

Thanks for opening this issue, it's very useful to see what issues users run into 😄

@likeamahoney
Copy link
Author

Hi @likeamahoney

Thanks for checking out SystemVerilog simulation with Arcilator!

This lowering path is still in early development and the issue you are encountering is because of using negedge while we currently only support posedge upstream (because that's what Chisel/FIRRTL uses). The fix is simple: just add a lowering pattern for seq.clock_inv to LowerArcToLLVM.cpp.

I have a branch with some fixes and additional features (including that one) that should be enough to fully lower your example, if you want to give it a try. I hope to upstream that sometime soon. On the branch, you can simply run arcilator directly after circt-verilog because the LLHD pipeline was added to circt-verilog.

Thanks for opening this issue, it's very useful to see what issues users run into 😄

Hi!

Much thanks! I tested your branch and I managed to translate all my examples related to triggers and sequential logic.

Hope it would be merged in main branch soon!

But combinational logic does not translate so well.

Combinational examples with continuous assignments only are lowered correctly:

For example:

module combo (  input   a, b, c, d, e, output  z);
        assign z = ((a & b) | (c ^ d) & ~e);
endmodule

But arcilator emits an error (error: 'llhd.process' op has regions; not supported by ConvertToArcs) when combinational logic is inside an always block. For example:

module combo (  input   a, b, c, d, e,  output  reg z);

        always @ (*) begin
                z = ((a & b) | (c ^ d) & ~e);
        end

endmodule

@maerhart
Copy link
Member

Thanks for pointing this out. I created an issue tracking this: #8013

In the meantime, you can use always_comb instead.

@likeamahoney
Copy link
Author

Thanks for pointing this out. I created an issue tracking this: #8013

In the meantime, you can use always_comb instead.

Thanks a lot!

Can I leave this issue open for future lowering issues reports or should I to close it?

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

No branches or pull requests

2 participants