tests: check that elaboration happens only once

[oharboe]

Build Dependencies

Build with: git-clone firrtl https://github.com/freechipsproject/firrtl.git master
Build with: git-clone chisel3 https://github.com/freechipsproject/chisel3.git master

[ducky64]

Thanks - the test looks structurally sane, with some style comments inline.

I think this is a useful test case, though fixing it is another thing entirely. The Verilator backend is largely legacy imported code, so it's unclear how easy fixing it will be.

If it's currently failing: I'm not sure I want a test that might stay failing for a while, maybe adding an ignore tag would be a compromise that keeps this issue somewhat visible but wouldn't block something like CI (if we started running it)?

[ducky64]

Can you have the variable increment inside the Module body itself, like

test(new Module {
  checkOnceVerilator should be (0)
  checkOnceVerilator += 1
}) { c =>
  // empty test body
}

You can also use the test(...).withAnnotations(...) { c => ... } syntax as demonstrated in VerilatorBasicTests.scala, which I think is cleaner than creating a TestBuilder object?

[oharboe]

test() syntax can't be used, because it doesn't take a producer of a module, but an actual module.

[oharboe]

Can you have the variable increment inside the Module body itself, like

That breaks the test...

[ducky64]

Wait, why does it break the test? test(...) is call-by-name on the DUT argument (essentially it's the same as () => ... but without the explicit syntax), and also I don't think it's allowed to (should assert out) elaborate a Chisel module outside a special context (that gets initialized inside somewhere test(...))

[oharboe]

I dont know exactly. I am using vscode and it doesnt support debugging yet . Also, I am learning Scala in spare moments so my understanding is shaky. ons. 18. des. 2019, 20:28 skrev Richard Lin <notifications@github.com>:

…

***@***.**** commented on this pull request. ------------------------------ In src/test/scala/chiseltest/tests/SingleElaboration.scala <#98 (comment)> : > +class SingleElaborationTests extends FlatSpec with ChiselScalatestTester with Matchers { + + behavior of "elaboration should happen once" + + class ThingAMaBobBundle extends Bundle { + val condition = Input(Bool()) + } + + class ThingAMaBob extends Module { + val io = IO(new ThingAMaBobBundle) + val counter = RegInit(0.U(2.W)) + } + + var checkOnceVerilator = 0 + it should ("elaborate once with Verilator") in { + new TestBuilder(() => { Wait, why does it break the test? test(...) is call-by-name on the DUT argument (essentially it's the same as () => ... but without the explicit syntax), and also I don't think it's allowed to (should assert out) elaborate a Chisel module outside a special context (that gets initialized inside somewhere test(...)) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#98>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVLJZUJ2RWEGQAARKEMBJ3QZJ2V5ANCNFSM4J2J6SBQ> .

[oharboe]

@ducky64 I think that there is a unit-test that verifies that elaboration doesn't happen twice? Is there? Can I close this PR?

[oharboe]

I can re-open if this still has any value.