Project

General

Profile

[logo] 
 
Home
News
Activity
About/Contact
Major Tools
  Dinotrace
  Verilator
  Verilog-mode
  Verilog-Perl
Other Tools
  BugVise
  CovVise
  Force-Gate-Sim
  Gspice
  IPC::Locker
  Rsvn
  SVN::S4
  Voneline
  WFH
General Info
  Papers

Issue #467

Support old format $display ($time, "...")

Added by Alex Solomatnikov almost 5 years ago. Updated about 2 months ago.

Status:
Closed
Priority:
Low
Assignee:
-
Category:
Unsupported
% Done:

0%


Description

Code:

$display ($time, "ps Error: STRATIX or STRATIXGX in non DPA mode does not support the specified deserialization factor!");

error:

syntax error, unexpected $time, expecting STRING or ')'

verilator-issue-467-wip.patch View (11.3 KB) Jamey Hicks, 10/21/2015 04:47 PM

verilator-bug467.patch View (3.9 KB) Jamey Hicks, 10/26/2015 03:17 PM

verilator-bug467.patch View (3.93 KB) Jamey Hicks, 10/26/2015 03:41 PM

History

#1 Updated by Wilson Snyder almost 5 years ago

  • Subject changed from old format $display ($time, "...") is not supported to Support old format $display ($time, "...")
  • Category set to Unsupported
  • Status changed from New to Feature

Obviously Verilator currently expects a format string.

#2 Updated by Alex Solomatnikov almost 5 years ago

Another example:

$display($realtime, "ps Warning: Inclock_Period Violation");

#3 Updated by Alex Solomatnikov almost 5 years ago

More $display() cases that don't work with verilator:

$display("Error!  lpm_representation value must be \"SIGNED\" or \"UNSIGNED\".", $time);

verilator output:

%Error: ...: Extra arguments for $display format

Another case:

$display("Time: %0t", $time, "   Warning : Detected transition on SCANCLK during quiet time. PLL may not function correctly."); 

#4 Updated by Wilson Snyder over 1 year ago

See also bug980.

Patches are welcome; at one point the code didn't discriminate between a user-string "..." and a number which just happened to print as a string, so it couldn't know if $display("foo") should print as a number or string. Some of that is now better, but it's likely still a problem in some places.

Also it's better to make a large format string than to break it up, because Verilator knows how to constant propagate into displays, so ideally a large $display will become in the output just a constant string. This doesn't work across multiple displays.

#5 Updated by Jamey Hicks over 1 year ago

I've started working on this. I see what you mean about constant propagation into display statements.

One question I have is how to tell if one of the arguments (other than the first) is a string.

I have this as input:

$write("%h", a, ">");

The dtype for the node for the ">" dumps:

BASICDTYPE 0x2687090 <e17679> {g3222} dt=this(G/w8)  logic [GENERIC] kwd=logic range=[7:0]

Is there some clue that this was a string?

#6 Updated by Jamey Hicks over 1 year ago

My current plan is to extend the format string in LinkResolveVisitor with appropriate format % operators for the extra nodes.

#7 Updated by Jamey Hicks over 1 year ago

Found V3Number::isFromString(), answering my own question.

#8 Updated by Jamey Hicks over 1 year ago

Attached is a work in progress to address this issue. I'll remove all the chatter from the final patch.

As I append strings found in the argument list to the format string, I need to remove those strings from the argument list. Is argp->unlinkFrBack() the right thing to call?

I'll work on some tests and then clean it up.

#9 Updated by Wilson Snyder over 1 year ago

Generally very good. Just a few nits.

+++ b/src/verilog.y
-       |       yD_FCLOSE '(' idClassSel ')'            { $$ = new AstFClose($1, $3); }
+       |       yD_FCLOSE '(' expr ')'                  { $$ = new AstFClose($1, $3); }
        |       yD_FFLUSH parenE                        { $1->v3error("Unsupported: $fflush of all handles does not map to C++."); }
-       |       yD_FFLUSH '(' idClassSel ')'            { $$ = new AstFFlush($1, $3); }
+       |       yD_FFLUSH '(' expr ')'                  { $$ = new AstFFlush($1, $3); }

Assume some tests will show the need for this, and that the change works OK.  Please provide as a separate patch.

+++ b/src/V3LinkResolve.cpp

-    void expectFormat(AstNode* nodep, const string& format, AstNode* argp, bool isScan) {
+    void expectFormat(AstNode* nodep, const string& format, AstNode* argp, bool isScan, string *newFormatp = 0) {

Please pass newFormat as a reference (newFormatr - r means reference).  Better, it can just be the return value.

+               int isFromString = 0;

bool

+                   int inpercent = 0;

bool inpercent = false;  (likewise "= true" below)

+                   newFormat.append(", %h");

newFormat += ", %h";

#10 Updated by Jamey Hicks over 1 year ago

Thanks for the feedback. Attached is the updated patch.

#11 Updated by Jamey Hicks over 1 year ago

One more update which supersedes the last patch.

#12 Updated by Wilson Snyder over 1 year ago

Great, pushed to git towards 3.878.

(Made one trivial change to add a dataByte() accessor to V3Number, which seemed cleaner.)

#13 Updated by Wilson Snyder over 1 year ago

  • Status changed from Feature to Closed

In 3.878.

#14 Updated by John Demme 2 months ago

Hi Guys-

Sorry, but from reading this exchange it's difficult for me to tell what's been fixed. Is the following now supported? $display ($time, "ps Error: STRATIX or STRATIXGX in non DPA mode does not support the specified deserialization factor!");

I ask since similar things in altera_mf.v don't seem to work in the latest master. (This appears to be the last barrier to altera_mf.v parsing!)

~John

#15 Updated by Wilson Snyder about 2 months ago

The $display($time) is fixed in git towards 3.891.

#16 Updated by John Demme about 2 months ago

Excellent! Thanks

Also available in: Atom