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

[SVExtractTestCode] Instance inlining doesn't update inner symbols and clone sv.bind #5665

Closed
uenoku opened this issue Jul 24, 2023 · 1 comment · Fixed by #5679
Closed
Labels
bug Something isn't working Verilog/SystemVerilog Involving a Verilog dialect

Comments

@uenoku
Copy link
Member

uenoku commented Jul 24, 2023

circuit Top:
  module Assert:
    input clock: Clock
    input a: UInt<1>
    assert(clock, a, a, "foo")

  module Top:
    input clock: Clock
    input a: UInt<1>
    input b: UInt<1>
    inst a1 of Assert
    a1.a <= a
    a1.clock <= clock
    inst a2 of Assert
    a2.a <= b
    a2.clock <= clock

The current output with firtool -extract-test-code.

// Generated by CIRCT firtool-1.48.0-44-gf453a87cd
// VCS coverage exclude_file
module Assert_assert(
  input a,
        clock
);

  always @(posedge clock) begin
    if (a)
      assert(a) else $error("foo");
  end // always @(posedge)
endmodule

module Top(
  input clock,
        a,
        b
);

endmodule


// ----- 8< ----- FILE "bindfile.sv" ----- 8< -----

// Generated by CIRCT firtool-1.48.0-44-gf453a87cd
bind Top Assert_assert Assert_assert (
  .a     (a),
  .clock (clock)
);

The second assertion is not emitted in the bind file. This is because bind op is not cloned properly when input only modules are inlined and the first instance (in the instance graph ndoe iterator) is preserved. This bug was first introduced by 0d5cf22 which was included in 1.17.0.

@dtzSiFive dtzSiFive added bug Something isn't working Verilog/SystemVerilog Involving a Verilog dialect labels Jul 24, 2023
@uenoku
Copy link
Member Author

uenoku commented Jul 25, 2023

Another example

circuit Top:
  module Assert:
    input clock: Clock
    input a: UInt<1>
    assert(clock, a, a, "foo")

  module Assert_wrapper:
    input clock: Clock
    input a: UInt<1>
    output b: UInt<1>
    inst as of Assert 
    as.a <= a
    as.clock <= clock
    b <= a

  module Top:
    input clock: Clock
    input a: UInt<1>
    input b: UInt<1>
    output c: UInt<1>
    inst a1 of Assert_wrapper
    a1.a <= a
    a1.clock <= clock
    inst a2 of Assert
    a2.a <= b
    a2.clock <= clock
    c <= a1.b

@uenoku uenoku changed the title [SVExtractTestCode] Instance extraction doesn't update inner symbols and clone sv.bind [SVExtractTestCode] Instance inlining doesn't update inner symbols and clone sv.bind Jul 25, 2023
uenoku added a commit that referenced this issue Jul 26, 2023
…ner symbols and clone sv.bind

Close #5665. This commit fixes a bug that
bound instances are accidentally erased. When inlining input only modules it's necessary
to clone bind statements for bound instances in the module. Previously single bind op is shared by multiple instances, as a result ExportVerilog only emits a bind statement for one of them.
uenoku added a commit that referenced this issue Jul 26, 2023
…nner symbols and clone sv.bind (#5679)

Close #5665. This commit fixes a bug that
bound instances are accidentally erased. When inlining input only modules it's necessary
to clone bind statements for bound instances in the module. Previously single bind op is shared by multiple instances, as a result ExportVerilog only emits a bind statement for one of them.
uenoku added a commit that referenced this issue Jul 31, 2023
…nner symbols and clone sv.bind (#5679)

Close #5665. This commit fixes a bug that
bound instances are accidentally erased. When inlining input only modules it's necessary
to clone bind statements for bound instances in the module. Previously single bind op is
shared by multiple instances, as a result ExportVerilog only emits a bind statement for one of them.
uenoku added a commit that referenced this issue Aug 2, 2023
…nner symbols and clone sv.bind (#5679)

Close #5665. This commit fixes a bug that
bound instances are accidentally erased. When inlining input only modules it's necessary
to clone bind statements for bound instances in the module. Previously single bind op is
shared by multiple instances, as a result ExportVerilog only emits a bind statement for one of them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Verilog/SystemVerilog Involving a Verilog dialect
Projects
None yet
2 participants