Jump to content

Philipp A Hartmann

Members
  • Content Count

    534
  • Joined

  • Last visited

  • Days Won

    129

Posts posted by Philipp A Hartmann


  1. Hi Patrick,

    right now, there is no official publicly accessible repository of the SystemC proof-of-concept implementation.  Both repos you mention above are not owned by Accellera.  So applying the patch manually on top of a local source tree is sufficient:

    cd systemc-2.3.2
    patch -p1 < 0001_sc_process_*.patch
    
    # usual install steps
    mkdir -p objdir
    cd objdir
    ../configure
    make
    make install

    Thanks,
      Philipp


  2. On 4/30/2018 at 6:30 PM, sheridp@umich.edu said:

    On the other hand, it might be nice for debuggability if waiting on an empty sc_event_and_list didn't wait at all or threw an exception.

    In fact, IEEE 1666-2011 already requires an error when calling wait/next_trigger with an empty event list (5.2.17, 5.2.18).  This is currently not the case for the proof-of-concept implementation and will be fixed in the next version.

    Unfortunately, I have not been able to reproduce the crash using your example on any of my setups.  Can you please try if the attached patch fixes the crash for you?

    Thanks,
      Philipp

    0001-sc_-_process-remove-dynamic-processes-from-process_t.patch


  3. 20 hours ago, sheridp@umich.edu said:

    The fix could be as simple as documenting that sc_spawn'ing in start_of_simulation is not allowed.

    IEEE 1666-2011 explicitly allows spawning threads from start_of_simulation, see 4.4.3 (b).

    19 hours ago, sheridp@umich.edu said:

    On the other hand, it might be nice for debuggability if waiting on an empty sc_event_and_list didn't wait at all or threw an exception.

    I agree.  Having some way to decide on the behavior on an empty event list would be helpful.  I'll forward your suggestions to the LWG.

    19 hours ago, sheridp@umich.edu said:

    Also, if I leave everything the same but switch out start_of_simulation for end_of_elaboration, I still get the segfault.

    The bug depends on the order of additions to the process table (because the static process' destructor will walk the table until it finds a match).  If I comment out either run3 or the spawn in run2, I don't see the segfault.  Hopefully this is helpful in finding a solution.

    Thanks for preparing the example!  I'll try to reproduce and investigate further.

    Greetings from Duisburg,
      Philipp


  4. Some further comments:

    • You don't need to keep handles to processes, if you don't intend to interact with them later.
    • The crash you saw was likely caused by the fact, that the process was killed before you tried to register the sensitivity to it through your sc_event_and_list. Even if you kept the handle alive, you would still miss the notification: the process has already terminated when you run your wait(processes_complete); and the event will not be triggered again.
    • It would be much safer to only store the handles and build up the event list locally to wait for the completion of still running processes:
      std::vector<sc_process_handle> processes;
      
      for(int i = 0; i < 10; ++i){
          auto process = sc_core::sc_spawn(sc_bind(&My_Class::my_method, this, args));
          processes.push_back(process);
          wait(10, SC_US);
      }
      
      // wait for running processes
      {
        sc_event_and_list processes_running;
        for( const auto& p : processes ) {
          if ( p.valid() && !p.terminated() )
            processes_running &= p.terminated_event();
        }
        wait( processes_running );
      }

       

    For the second part (crash during process cleanup):

    • Do you see a difference when spawning the thread during end_of_elaboration()?
    • sc_spawn'ed threads from an start_of_simulation() callback a strictly speaking "dynamic threads" in the SystemC terminology, yes.  But internally, they are still added to the process table in order to start them in the correct order relative to the rest of the kernel initialization.  In the destructor however, only non-dynamic threads are removed from the process table.  This means, you may have indeed found a bug in SystemC 2.3.2.

    I'll check this with the Language Working Group.

    Thanks and Greetings from Duisburg,
      Philipp


  5. On the first question:

    Yes, you do need to keep the handle to the (terminating) process alive, if you continue to reference any related object like the terminated event.  Otherwise, when the process terminates without any existing handles to it, the event will be destroyed together with the process instance itself and you're sensitive to a no-longer-existing event, causing the memory corruption.  The crash itself is fixed in the master branch of the SystemC proof-of-concept simulator, but not released yet.  This fix would then lead to removing the event from any waiting processes, though.  So in this case, you would just miss the notification.

    On the second part:

    Which version of SystemC are you using?  Can you confirm this with SystemC 2.3.2?

    Greetings from Duisburg,
      Philipp


  6. On 23/03/2018 at 12:54 AM, Eyck said:
    
    My::My(sc_core::sc_module_name nm, int ID)
        : sc_core::sc_module(concatenate(nm, ID))

     

    This usage of sc_module_name is wrong and will break the module hierarchy.

    The only thing you are allowed (but not even required) to do with the module name object inside your constructor is to forward it to the sc_module base class.

    As Roman said, you would have to prepare the name outside of the module instead (or use sc_vector).

    /Philipp


  7. 6 hours ago, Mouhsen.Ibrahim said:

    Is there a change log for systemc, so I can see what changed and modify my systemc models according to it?

    Annex C in the SystemC standard lists some deprecated features.

    In the SystemC proof-of-concept implementation, you find detailed information about changes in the RELEASENOTES file, but mostly only relative to the previous version.


  8. I agree with your conclusion that the observed behavior of the proof-of-concept implementation does not match the requirements of IEEE 1666-2011. I checked the code and it can be fixed by adding the check for resets to sc_thread_process.h (in the trigger_static() function):

    diff --git a/src/sysc/kernel/sc_thread_process.h b/src/sysc/kernel/sc_thread_process.h
    --- a/src/sysc/kernel/sc_thread_process.h
    +++ b/src/sysc/kernel/sc_thread_process.h
    @@ -485,5 +486,5 @@ sc_thread_process::trigger_static()
     #endif // SC_ENABLE_IMMEDIATE_SELF_NOTIFICATIONS
     
    -    if ( m_wait_cycle_n > 0 )
    +    if ( m_wait_cycle_n > 0 && THROW_NONE == m_throw_status )
         {
             --m_wait_cycle_n;


    I'll take this change to the language working group to get it fixed in a future version of the SystemC PoC kernel. Thanks for reporting!

    Greetings from Duisburg,
      Philipp
     


  9. Hi Ameya,

    thanks for the feedback!  Can you please provide the following details:

    • Platform (Cygwin?)
    • GCC version used
    • Logs of verify.pl with -v enabled (to see the full command-lines)

    The file cci_core/systemc.h is an internal header which is only included through this path.  A conflict could only occur, should you add cci_core/ directly to the compiler include path (which is not supported), or if you'd install SystemC below cci_core/ (which would be very weird ;-)).  

    The errors in ex07_Parameter_Information indeed look like the SystemC installation is not correctly resolved.  For this, the full command-lines would really help.

    The error in ex16_User_Defined_Data_Type is indeed a bug (at least before C++14(?)).  The fix is to explicitly open the cci namespace around the cci_value_converter specialization in ex16_user_datatype.h.

    Hope that helps and thanks again,
      Philipp


  10. Hi Fred,

    synchronous resets are indeed not triggering the process themselves (that's why it's called "synchronous) and instead are only checked when the process is triggered through other means.  Typically, this would be a trigger from the static sensitivity (clk.pos() in your case).

    As @AmeyaVS correctly pointed out, dynamic sensitivity replaces the triggering behavior (not the reset specs).  So your synchronous reset would be checked only, when some_signal is de-asserted.  Asynchronous resets continue to fire.

    A polling loop would be the most accurate way to model your design wrt. resets. If you know the reset signals at the point of that wait statement, you can approximate the behavior by waiting for either the signal or the reset (which will then reset the process)

    wait( some_signal.negedge_event() | rst_n.negedge_event() );

    Of course, this might be one clock cycle off.  To be correct, you need to have an event that's synchronous to the clock again:

    sc_event thread_rst_ev;
    
    SC_METHOD(thread_sync_reset);
      sensitive << clk.pos();
    
    // ...
    
    void thread_sync_reset() {
      if (rst_n == false)
        thread_rst_ev.notify();
    }
    
    // ...
    wait( some_signal.negedge_event() | thread_rst_ev );

    By the way: a smart wait_until( some_signal == false ), which supports (synchronous) resets and avoids polling internally would be a very useful feature for writing signal-level protocols.

    Greetings from Duisburg,
      Philipp


  11. First of all, we need to distinguish between SystemC in general and its synthesizable subset.  Whenever something is not explicitly covered by the current synthesizable subset, it's probably best to include the HLS vendors in the discussion.  This almost certainly includes sc_signal<sc_uint_base>.  Vendors may support this as signal type, if they can derive the width of the signal (statically) during elaboration.  But in a fully generic fabric model, this might be difficult.

    IIRC, the main difference between SystemC and MyHDL or Chisel is that the latter basically create an explicit "netlist" in memory during elaboration.  Basically, the synthesis rules/tool is already embedded in the language (kernel) itself, knowing how to generate the elaborated model to Verilog from memory.  In SystemC, a synthesis tool is implemented more like a compiler, i.e. working at the language level directly.  These are very different approaches with different benefits and limitations.

    When you refer to "runtime-defined bitwidths", you certainly mean "elaboration-defined", because the widths cannot change once the simulation has started.  But sc_uint_base doesn't know about elaboration/simulation. In order to reduce the risk of width mismatches, you would need a custom channel as suggested by Torsten.  But as you said, such custom channels are not supported well by HLS tools.

    Long story short: Having an "elaboration-sized" signal could definitely help for HLS use cases like yours.  The simulation model could be implemented quite easily based on a signal specialization for sc_(u)int_base/sc_(u)nsigned.  But as long as HLS tools doesn't support it, its usage would be fairly limited. Have you talked to HLS vendors about this already?

    Last, but not least, I think that C++17 constexpr, especially in combination with if constexpr and auto return type deduction, will provide most of the required features to write generic "hardware construction code" efficiently.  Such models would then automatically be synthesizable by an C++17 capable HLS tool, as the code is evaluated during compile time already.


  12. This question in mostly about how the linker works on your platform, and not really specific to SystemC.  Let me try to give a short summary:

    • Yes, the "main" symbol is needed by the final application
    • For every needed symbol, the linker looks in your object files first
      • If the symbol is present there, the linker picks it up
      • If not, the linker looks in the libraries given by the user (order/lookup is platform-specific)
    • Repeat this for all required symbols (including "sc_main")

    So, if you define your own "main" in your application, SystemC's "main" will not be picked. This is actually a feature, as you can call "sc_elab_and_sim" manually from your "main" as well, if you want.

    Hope that helps,
      Philipp

     


  13. 13 hours ago, ANKUR SAINI said:

    Why it has not called the analyzer before_end_of_elaboration() callback ?

    Because your "analyzer" instance is a local variable in the before_end_of_elaboration function, which gets destructed immediately, when the function returns.  So there is no analyzer left to call the callback on later (same holds for the local signal sig2, btw.).  You would need to allocate the module dynamically instead.

     

    13 hours ago, ANKUR SAINI said:

    Also, can I alter/modify/redefine the port bindings in the top level before_end_of_elaboration callback (see the commented lines in top level callback)?

    You cannot alter/modify/redefine any existing port binding at any time.  You can only add new bindings, including during before_end_of_elaboration.

    Hope that helps,
      Philipp


  14. 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.


  15. 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


  16. Hi Jarodw,

    Thanks for your report. I can confirm and reproduce the issue in SystemC 2.3.2.

    It looks indeed like a regression compared to SystemC 2.3.0/1 that has been introduced by the fix for optionally unbound sockets, see:

    It seems, the SystemC regression tests didn't cover the hierarchical binding for the multi sockets, so it wasn't caught before the release.

    Your example can be fixed by changing line 228 in src/tlm_utils/multi_passthrough_target_socket.h:

      if (unbound && !m_hierarch_bind) return;
      //          ^-- add check for hierarchical binding here

    Hope that helps,
      Philipp


  17. In addition to Thorsten's comments, your main problem is that you call the processes directly instead of invoking sc_start.  You should never call process functions manually.  They are  scheduled by the kernel during the simulation instead.

    Here's a cleaned up version of your sc_main:

    int sc_main(int /*argc*/, char* /*argv*/[])
    {
      sc_signal<double> force_main("force main");
      sc_signal<double> area_main("area_main");
    
      stimuli_pressure_mech S("stimuli_pressure_mech");
        S.force_out(force_main);
        S.area_out(area_main);
    
      pass_on_impulse PI("pass_on_impulse");
        PI.force_in(force_main);
        PI.area_in(area_main);
    
      sc_start();
      return 0;
    } 

    I did the following changes:

    • Drop warning suppression - why do you want to allow deprecated features? They are deprecated for a reason.
    • Add names to your signals
    • Drop dynamic allocation of modules in favor of properly cleaned up local variables
    • Drop manual calls to processes
    • Call sc_start to start the simulation (without specific end time, but Thorsten's suggestions is also possible)

    Hope that helps,
      Philipp

×
×
  • Create New...