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

-x-initial-edge breaks with logic bug fix #604

Closed
veripoolbot opened this issue Jan 17, 2013 · 3 comments
Closed

-x-initial-edge breaks with logic bug fix #604

veripoolbot opened this issue Jan 17, 2013 · 3 comments

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Jan 17, 2013


Author Name: Wilson Snyder (@wsnyder)
Original Redmine Issue: 604 from https://www.veripool.org
Original Date: 2013-01-17
Original Assignee: Jeremy Bennett (@jeremybennett)


Part of the fix for #� uncovered a bug I had in V3AstNodes.cpp. The git head now includes these lines:

 AstBasicDType* new1p = new AstBasicDType(fl, AstBasicDTypeKwd::BIT, numeric, width, widthMin);
 // Above should be below, but fails --x-initial-edge test
 //AstBasicDType* new1p = new AstBasicDType(fl, kwd, numeric, width, widthMin);

The use of ::BIT here should have been kwd. This bug means some logic types were mis-converted to bit types.

However fixing this breaks the t_initial_edge test.

Can you look into this?

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 17, 2013


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2013-01-17T01:55:10Z


#�_initial_edge branch contains the necessary edits, as another test needed a fixup too.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 17, 2013


Original Redmine Comment
Author Name: Jeremy Bennett (@jeremybennett)
Original Date: 2013-01-17T10:45:38Z


The test for when to implement --x-initial-edge in EmitCImp::emitVarResets() in V3EmitC.cpp was wrong.

index 5634055..e148921 100644
--- a/src/V3EmitC.cpp
+++ b/src/V3EmitC.cpp
@@ -1368,17 +1368,16 @@ void EmitCImp::emitVarResets(AstNodeModule* modp) {
                 } else {
                     puts(varp->name());
                     for (int v=0; v<vects; ++v) puts( "[__Vi"+cvtToStr(v)+"]");
-                   if (zeroit) {
-                       // We want to force an initial edge on uninitialized clocks (from 'X' to
-                       // whatever the first value is). Since the class is instantiated before
-                       // initial blocks are evaluated, this should not clash with any initial
-                       // block settings. Clocks are always BIT datatypes, so zeroit is true.
-                       if (v3Global.opt.xInitialEdge()
-                           && (0 == varp->name().find("__Vclklast__"))) {
-                           puts(" = 1;\n");
-                       } else {
-                           puts(" = 0;\n");
-                       }
+                   // If --x-initial-edge is set, we want to force an initial
+                   // edge on uninitialized clocks (from 'X' to whatever the
+                   // first value is). Since the class is instantiated before
+                   // initial blocks are evaluated, this should not clash
+                   // with any initial block settings.
+                   if (v3Global.opt.xInitialEdge() && varp->isUsedClock()) {
+                       puts(" = 0;\n");
+                   } else if (v3Global.opt.xInitialEdge()
+                              && (0 == varp->name().find("__Vclklast__"))) {
+                       puts(" = 1;\n");
                     } else {
                         puts(" = VL_RAND_RESET_");
                         emitIQW(varp);

Please pull a corrected version (as a sub-branch of #�_initial_edge) from branch x-initial-edge at git@github.com:jeremybennett/verilator.git

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jan 17, 2013


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2013-01-17T12:21:14Z


Thanks for the quick fix. I added a || zeroit as otherwise the t_var_types test correctly noticed that a 2-state type wasn't zero initialized. This should be still compatible with other simulators as a two-state which inits to zero will NOT trigger a negedge, it's the X->0 that does.

Applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.