Skip to content
This repository has been archived by the owner on Aug 19, 2024. It is now read-only.

Fix vcs assertions #548

Merged
merged 1 commit into from
Aug 15, 2022
Merged

Fix vcs assertions #548

merged 1 commit into from
Aug 15, 2022

Conversation

terpstra
Copy link
Contributor

The 'reset' signal cannot glitch from 0 to 1 at power on.
If it does, then all the assertions in a vcs design will fire.

@ekiwi
Copy link
Collaborator

ekiwi commented Aug 13, 2022

Do you know why this is happening? Asserts should only be evaluated if there is a positive clock edge.

@terpstra
Copy link
Contributor Author

I did not delve into how the clock is generated by chisel-unit-tests. However, in the waveforms I saw, there was activity on the clock and reset before reset got pegged high.

@terpstra
Copy link
Contributor Author

I take it back. It only looks like there is activity when you zoom out too far in dve (the grey becomes yellow).
This is what it looks like in a waveform which triggered the asserts:
image

@ekiwi
Copy link
Collaborator

ekiwi commented Aug 15, 2022

Do you know what timestep the assert is triggered at?

@terpstra
Copy link
Contributor Author

BTW, both this kind of assert:

    `ifndef SYNTHESIS
    `ifdef PRINTF_COND
      if (`PRINTF_COND) begin
    `endif
        if (_GEN_16 & ~x15) begin
          $fwrite(32'h80000002,
            "Assertion failed: [verif-library-assert]<extraction-summary>{\"predicateModifier\":{\"type\":\"noMod\"},\"format\":{\"type\":\"ifElseFatal\"},\"labelExts\":[\"client_ag_address_no_server\"],\"baseMsg\":\"assert failed (verification library): 6.7.1.1.4.1 AG_address does not belongs to any logical server. AG_Address: 0x%x  @[Monitor.scala 139:11]\"}</extraction-summary>\n    at Property.scala:96 chisel3.assert(predicate, flag + exStr, msgArgs: _*)(sourceInfo, compileOptions)\n"
            ,io_in_ag_address); // @[Monitor.scala 139:11]
        end

and this kind of assert fire:

  always @(posedge clock) begin
    //
    if (_T_1 & _T_3) begin
      assert(_x29_muxDefault_T_4); // @[Monitor.scala 151:11]
    end

@terpstra
Copy link
Contributor Author

I see this in the output:

"ECFuzzerTest.sv", 44144: testbench.ECFuzzerTest.TL2cXbar.monitor.unnamed$$_4: started at 0ps failed at 0ps
	Offending '_x29_muxDefault_T_8'
"ECFuzzerTest.sv", 44148: testbench.ECFuzzerTest.TL2cXbar.monitor.unnamed$$_5: started at 0ps failed at 0ps
	Offending '_x29_muxDefault_T_8'
...
Assertion failed: AltQueue count exceeds capacity
    at AltQueue.scala:55 assert(io.count <= entries.U, "AltQueue count exceeds capacity")
Assertion failed: AltQueue count exceeds capacity
    at AltQueue.scala:55 assert(io.count <= entries.U, "AltQueue count exceeds capacity")

@terpstra
Copy link
Contributor Author

The assert statements tell you it was at 0ps. I am just assuming the PRINTF flavor are also asserting at time 0 (since it does not print it and it passed when I change the $reg reset = 1)

@ekiwi
Copy link
Collaborator

ekiwi commented Aug 15, 2022

The assert statements tell you it was at 0ps.

Can you see a rising edge on the clock at 0ps? Since these statements are in a always @posedge clock process, I would assume that they can only trigger if there is such an event. Not too familiar with Verilog edge-case semantics though, so correct me if I am wrong.

@terpstra
Copy link
Contributor Author

I have also set +define+PRINTF_COND=!testbench.reset in my VcsFlags. This is needed even when the reset powers on as 1.

@terpstra
Copy link
Contributor Author

Aha! When I include time 0 in the waveform, the grey becomes yellow. It's not about the zoom:
image

So, if I zoom in around 1000ps, I see grey for clock+reset. If I include 0ps, it shows as yellow (which suggests it toggled at 0).

@terpstra
Copy link
Contributor Author

No matter how far in I zoom at 0, I always see a solid yellow bar for clock+reset. I can never actually SEE the transition, because it's presumably instantaneous.

@terpstra
Copy link
Contributor Author

I mean, you should be able to reproduce this problem as easily as adding an assert and simulating it with vcs?

@ekiwi
Copy link
Collaborator

ekiwi commented Aug 15, 2022

@terpstra: could your problem be fixed by setting clock to 1 from the start instead of reset?
That would make me a lot happier since we always have a very good idea of which signal the clock is (since it is of Clock type), but reset is not quite as special since it does not need to be of a unique type in Chisel.

@terpstra
Copy link
Contributor Author

The clock already IS set to 1 by the -harness.sv, so I don't think that will help.

@terpstra
Copy link
Contributor Author

Hah! I just tried setting clock to 0; ie:

diff --git a/src/main/scala/chiseltest/simulator/ipc/VpiVerilogHarnessGenerator.scala b/src/main/scala/chiseltest/simulator/ipc/VpiVerilogHarnessGenerator.scala
index 513997e..e613609 100644
--- a/src/main/scala/chiseltest/simulator/ipc/VpiVerilogHarnessGenerator.scala
+++ b/src/main/scala/chiseltest/simulator/ipc/VpiVerilogHarnessGenerator.scala
@@ -24,7 +24,7 @@ private[chiseltest] object VpiVerilogHarnessGenerator {
 
     val codeBuffer = new StringBuilder
     codeBuffer.append(s"module $testbenchName;\n")
-    codeBuffer.append(s"  reg $clockName = 1;\n")
+    codeBuffer.append(s"  reg $clockName = 0;\n")
     toplevel.inputs.foreach { case PinInfo(name, width, _) =>
       codeBuffer.append(s"  reg[${width - 1}:0] $name = 0;\n")
     }

... and THAT fixed it also.

@ekiwi
Copy link
Collaborator

ekiwi commented Aug 15, 2022

Hah! I just tried setting clock to 0; ie:

I like that fix. Happy to merge if you can update this PR.

It seems that clock=1 at time zero qualifies as a posedge for vcs.
This caused assertions to fire as registers have not yet been reset.
Copy link
Collaborator

@ekiwi ekiwi left a comment

Choose a reason for hiding this comment

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

Thanks! :shipit:

@ekiwi ekiwi merged commit 541ae8c into ucb-bar:main Aug 15, 2022
@terpstra
Copy link
Contributor Author

I've confirmed that by applying/unapplying/applying this change that it does cause the bug to toggle off/on/off respectively.

@terpstra terpstra deleted the fix-vcs-assertions branch August 15, 2022 18:36
@terpstra
Copy link
Contributor Author

@jackkoenig Could we get this backported onto 9f408a5 ?

@ekiwi
Copy link
Collaborator

ekiwi commented Aug 15, 2022

@Mergifyio backport 0.5.x

mergify bot pushed a commit that referenced this pull request Aug 15, 2022
It seems that clock=1 at time zero qualifies as a posedge for vcs.
This caused assertions to fire as registers have not yet been reset.

(cherry picked from commit 541ae8c)
@mergify
Copy link
Contributor

mergify bot commented Aug 15, 2022

backport 0.5.x

✅ Backports have been created

ekiwi pushed a commit that referenced this pull request Aug 15, 2022
It seems that clock=1 at time zero qualifies as a posedge for vcs.
This caused assertions to fire as registers have not yet been reset.

(cherry picked from commit 541ae8c)

Co-authored-by: Wesley W. Terpstra <wesley@sifive.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants