Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-04-26T11:12:06Z

I'd suggest adding a stack, then on visit(AstNodeFTaskRef) push nodep to it. Pop at the bottom of that function after iterating. Then on error you can just walk the stack.

Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2017-05-08T12:29:31Z

Do you think there should be an option to turn this behavior on? I'll always want to see it, but it's possible that the resulting message could be very verbose. I'd like to think that when people get the stack trace, they'll always find it helpful. But I could also appreciate an argument for brevity by default.

Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2017-05-08T20:02:27Z

I've got stack tracing working and I added complex data type pretty printing (for some complex data types). Here's an example of a call stack involving an input parameter which is a packed array of structs which themselves have an element which is a struct:

Called from:
t/t_func_const_packed_struct_fatal2.v:42:  f_add() with parameters
     params = [0 = '{a: 32'h7, foo: 6'hb, sub_params: '{b: 32'h37, bar: 8'h6f}}, 1 = '{a: 32'h3039, foo: 6'hc, sub_params: '{b: 32'h8, bar: 8'h70}}]
Called from:
t/t_func_const_packed_struct_fatal2.v:19:  f_add2() with parameters
     a = ?32?sh7
     b = ?32?sh8
     c = ?32?sh9

I was initially going to put the pretty printer in V3Number, but V3Number does not currently depend on the V3Ast headers. It seemed like it would be better not to add that linkage for this pretty printer which will likely not be used outside of V3Simulate. But I can easily refactor if it's better in V3Number.

Also, I didn't add a command line parameter for this, but again if desired I can add that.

Other than those caveats, this should be ready to go:
https://github.com/toddstrader/verilator-dev/tree/stack_trace

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-05-08T23:52:14Z

I don't see a need for an option.

Instead of pretty number, please call something similar to V3Cdc.cpp's use of V3EmitV::verilogPrefixedTree. It's fine if you need to improve/extend V3EmitV.

Great testing BTW.

Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2017-05-09T15:58:39Z

Fascinating, I didn't know Verilator could emit Verilog. It looks like this is mainly used for debugging. Is that correct?

I'm sure I don't fully grok this class yet, but the core idea behind V3EmitV might not be what we want here. Is it fair to say that V3EmitV's job is to take a portion of the AST and turn it into Verilog which fully represents that portion of the AST?

At this point in V3Simulate we know the port and pin for a function call and can get the V3Number out of the pin:
https://github.com/toddstrader/verilator-dev/blob/stack_trace/src/V3Simulate.h#L189

To try to understand your suggestion, I started V3EmitV'ing things while running the t_func_const_packed_struct_fatal2.pl test. For f_add2() things are pretty straightforward. V3EmitV'ing pinp for input parameter "a" gives me:

'sh7

However, doing the same for "params" in f_add() gives me:

params

This is somewhat confusing because I named the local variable and the function's parameter the same thing. I changed the name of the local variable to the_params and then got this from V3EmitV:

the_params

The problem is (again, if I understand correctly) we could use V3EmitV to kick out something like this:

f_add(the_params);

to show how f_add() was called and what was passed into it. But once we use variables with functions, we can't immediately emit a useful literal for the user in a stack trace.

I guess V3EmitV could be taught to look for user3p from V3Simulate. Do other things call V3EmitV which also use user3p? If not (and if this is safe), perhaps V3EmitV could kick out the literal representation of the_params instead of "the_params" itself if user3p were available. I'd basically be moving prettyNumber() into the AstVarRef/AstVarXRef visitors. Would this be desirable? I'm not clear on what all of the ramifications might be. Also, am I just looking at this all wrong? Is there a better way to proceed here?

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-05-09T17:09:00Z

V3EmitV is only used by CDC at present, so may have bugs where it isn't very verbose.

Is it fair to say that V3EmitV's job is to take a portion of the AST and turn it into Verilog which fully represents that portion of the AST?

Yes.

I'm not sure if there is a better way to proceed, I was just thinking of removing duplication. Maybe it helps, especially dealing with more complicated types in the future or maybe not and it can stay where is.

Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2017-05-09T17:44:03Z

OK, then I would advocate leaving prettyNumber() in V3Simulate since I think it has more affinity with that code than with V3EmitV. I had the same thoughts about the connection between this and deeper type knowledge in Verilator. But since I haven't looked deeply at that issue, I can't say if we could leverage this for later on.

If you buy that, then I'd say this patch is still ready to go:
https://github.com/toddstrader/verilator-dev/tree/stack_trace

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-05-09T22:56:01Z

Pushed to git towards 3.903.

BTW made a minor change to use _bad for tests that have warnings instead of _fatal, just to match other tests.

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-05-31T02:06:55Z

In 3.904.