Guedes Posted December 5, 2017 Report Share Posted December 5, 2017 I think I just found a bug at the file sc_trace_file_base.cpp on version 2.3.2. It was recommended to me to report it here.The problem is at the function set_time_unit, line 193, where you can read that the parameters of the function are v and tu: void sc_trace_file_base::set_time_unit( double v, sc_time_unit tu ) As you can see, there are no restriction for the use of value v. On the code I was working at, the value was 0.5, and that is ok for SystemC 2.3.1a. However, at 2.3.2, any value of v that it is not a multiple of 10 will make the function fail in an internal assertion because trace_unit_fs = v * unit_to_fs(tu): trace_unit_fs = static_cast<unit_type>(v * unit_to_fs(tu)); Then, at the line 213, fs_unit_to_str is called: ss << fs_unit_to_str(trace_unit_fs) And in fs_unit_to_str: it:std::string sc_trace_file_base::fs_unit_to_str(sc_trace_file_base::unit_type tu) { switch (tu) { case UINT64_C(1): return "1 fs"; case UINT64_C(10): return "10 fs"; case UINT64_C(100): return "100 fs"; case UINT64_C(1000): return "1 ps"; case UINT64_C(10000): return "10 ps"; case UINT64_C(100000): return "100 ps"; case UINT64_C(1000000): return "1 ns"; case UINT64_C(10000000): return "10 ns"; case UINT64_C(100000000): return "100 ns"; case UINT64_C(1000000000): return "1 us"; case UINT64_C(10000000000): return "10 us"; case UINT64_C(100000000000): return "100 us"; case UINT64_C(1000000000000): return "1 ms"; case UINT64_C(10000000000000): return "10 ms"; case UINT64_C(100000000000000): return "100 ms"; case UINT64_C(1000000000000000): return "1 sec"; case UINT64_C(10000000000000000): return "10 sec"; case UINT64_C(100000000000000000): return "100 sec"; default: sc_assert(0); return ""; } } If the variable v is different of a multiple of 10, the function fs_unit_to_str will necessarily fail. For v = 0.5, it will always get to the default and sc_assert(0) will be activated. This function should receive the value of unit_to_fs(tu) as a parameter, or change the way this block of code works altogether. Could someone confirm if this is a bug indeed, and propose a fix for that? Thank you very much. Quote Link to comment Share on other sites More sharing options...
Roman Popov Posted December 5, 2017 Report Share Posted December 5, 2017 Thanks a lot for reporting! I can confirm that 2.3.2 does not supports v that it is not a multiple of 10. I agree that set_time_unit API is bad (error-prone), and should be changed. Good news that you've received a runtime error early and did not spent lots of time . Do you need support for fractional timeunits? Or just more clear API? I've also checked 2.3.1. It does not works properly with 0.5 either: int sc_main (int argc, char **argv) { sc_trace_file* tf; tf = sc_create_vcd_trace_file("traces"); sc_signal<int> isig{"isig",0}; tf->set_time_unit(0.5, SC_NS); sc_trace(tf, isig, "isig" ); sc_start(0.5, SC_NS); isig = 1; sc_start(2.5, SC_NS); isig = 2; sc_start(3.5, SC_NS); isig = 4; sc_start(3.5, SC_NS); sc_close_vcd_trace_file(tf); return 0; } VCD is incorrect: $date Dec 05, 2017 11:50:52 $end $version SystemC 2.3.1-Accellera --- Dec 5 2017 11:40:15 $end $scope module SystemC $end $var wire 32 aaaaa isig [31:0] $end $upscope $end $enddefinitions $end $comment All initial values are dumped below at time 0 sec = 0 timescale units. $end $dumpvars b0 aaaaa $end #1 b1 aaaaa #6 b10 aaaaa #13 b100 aaaaa So 2.3.2 is better. But not perfect :( Guedes 1 Quote Link to comment Share on other sites More sharing options...
Philipp A Hartmann Posted December 5, 2017 Report Share Posted December 5, 2017 Quoting IEEE 1666-2011, 8.1.2 (color highlighting mine): Quote Member function set_time_unit shall be overridden in the derived class to set the time unit for the trace file. The value of the double argument shall be positive and shall be a power of 10. In the absence of any call function set_time_unit, the default trace file time unit shall be 1 picosecond. So I think, the implementation in SystemC 2.3.2 is in line with the SystemC standard. Instead of failing with an assertion, a more explanatory error message would be helpful, of course. Greetings from Duisburg, Philipp Guedes 1 Quote Link to comment Share on other sites More sharing options...
Roman Popov Posted December 5, 2017 Report Share Posted December 5, 2017 I've looked again at VCD generated by 2.3.1. It has no time unit mentioned at all. So it is actually correct : if you substitute 0.5 NS as a timeunit, you will have correct timestamps. Gtkwave renders them as 1ns. But other VCD viewers I've tried just don't show time unit at all in this case. Quote Link to comment Share on other sites More sharing options...
Roman Popov Posted December 5, 2017 Report Share Posted December 5, 2017 Quote $date Dec 05, 2017 12:21:02 $end $version SystemC 2.3.1-Accellera --- Dec 5 2017 11:40:15 $end $timescale 500 ps $end $scope module SystemC $end $var wire 32 aaaaa isig [31:0] $end $upscope $end $enddefinitions $end $dumpvars b0 aaaaa $end #1 b1 aaaaa #3 b10 aaaaa #7 b100 aaaaa GtkWave renders correctly with 500 ps timeunit. But commercial viewer I have crashes. Quote Link to comment Share on other sites More sharing options...
Philipp A Hartmann Posted December 5, 2017 Report Share Posted December 5, 2017 I can also add a quote from the Verilog Standard IEEE 1364-2005, 18.2.3.5 $timescale, describing the grammar rules for the timescale support in VCD: Quote vcd_declaration_timescale ::= $timescale time_number time_unit $end time_number ::= 1 | 10 | 100 time_unit ::= s | ms | us | ns | ps | fs So even here, only powers of 10 are allowed. Guedes 1 Quote Link to comment Share on other sites More sharing options...
Guedes Posted December 6, 2017 Author Report Share Posted December 6, 2017 I think a clear API should be fine, considering that IEEE 1666-2011, 8.1.2 explicitly defines it as a power of 10. Maybe an assert checking if (v % 10 == 0) would be interesting here, with a more direct message about the wrong parameterization. About my specific issue, I will change the code here properly. Thanks. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.