Jump to content

Philipp A Hartmann

Members
  • Content Count

    535
  • Joined

  • Last visited

  • Days Won

    130

Posts posted by Philipp A Hartmann

  1. This is not really a memory leak, but a very bad „model“ for the current implementation. I wonder if you saw any such scenario in a real model?

    „Canceled“ notifications like in your case will still be kept in the kernel‘s internal data structures until the notification time is reached (1ms in your case).  You would pile up 999,999,999 of these canceled notifications until they are „deallocated“, each of them taking entries in the event queue and 16 bytes for the notification itself. Which is a lot of memory.

    I wrote „deallocated“ in quotes, because sc_event_timed uses a very simple memory pool based on a free list to reuse previously allocated structures.  So even when the canceled notifications are released, they won‘t be handed back to the OS (not even at the end of the simulation) but will be kept allocated for future notifications.  But they do not leak in an unintended way.

    If you change your example to e.g. have the same number of iterations without piling up billions of pending events, you should see a stable memory consumption:

    event.notify( 1, SC_NS );
    event.notify( 1, SC_PS );

    Hope that helps,
     Philipp

  2.  

    22 hours ago, Eyck said:

    Actually there is a default time unit in sc_time.h

     

    Looking at the source code of any SystemC implementation is sometimes misleading. Check IEEE 1666-2011, Annex C (deprecated features), item (n):

    Quote

    n) Default time units and all the associated functions and constructors, including:

    1. Function sc_simulation_time
    2. Function sc_set_default_time_unit
    3. Function sc_get_default_time_unit
    4. Function sc_start(double)
    5. Constructor sc_clock(const char*, double, double, double, bool)

     

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

    This is exactly what I want to do, but next_trigger(sc_event_or_list&) is protected within sc_method_process.

    ... and sc_method_process is a non-standard class, which even is incomplete on the model side when including <systemc(.h)>. So I would not call this clean.

    Regarding your original solution, it is important to note that you are not supposed to change an sc_event_list object while processes are sensitive to it. In your case, it works because you trigger the process immediately, which then 're-reads' the event list right away and therefore updates the sensitivity without breaking the kernel state

    What do you want to achieve that makes you want to do this?

    /Philipp

  4. On 2/5/2020 at 9:46 AM, haase said:

    If i check the SystemC version 2.2, there was this function public, so it was allowed to use this function.

    The only normative reference for SystemC is defined by the IEEE Std. 1666-2011, see http://ieeexplore.ieee.org/document/6134619/. The is_reset() was never part any version of the standard, so one could never rely on its presence. You would to check, why the ReChannel library tries to call this function and then find another way to implement this functionality.

    /Philipp

  5. Actually, it is expected that you don't have the QuickThreads package built on MinGW.  As you can see from your config.log, the threading package to use is

    Quote

    ---------------------------------------------------------------------
    Configuration summary of SystemC 2.3.3 for x86_64-pc-mingw64
    ---------------------------------------------------------------------

    ...

     Build settings:
       Enable compiler optimizations  : yes
       Include debugging symbols      : no
       Coroutine package for processes: WinFiber
       Enable VCD scopes by default   : yes
       Disable async_request_update   : no
       Phase callbacks (experimental) : no
    --------------------------------------------------------------------

    But it seems that the source code is missing an explicit check for _WIN64 to catch the MinGW-64 case. Can you try compiling with WIN64 defined (without leading underscore):

    ../configure CXXFLAGS="-DWIN64"

    Hope that helps,
      Philipp

  6. This compiler warning is a false positive.

    There is a loop in sc_fifo<T>::read(T&) ensuring that the fifo is not empty (and the success of the nb_read(T&) is even guarded by an sc_assert😞

        while( num_available() == 0 ) {
    	sc_core::wait( m_data_written_event );
        }
        bool read_success = sc_fifo<T>::nb_read(val_);
        sc_assert( read_success );

    The check for num_available() is even stricter than the check in buf_read, but I can imagine that some compilers might not be able to prove this invariant.  Therefore, unconditionally initializing the local variable to silence the warning might be an acceptable trade-off.

  7. sim_input.h:344 and sim_sync.h:29 points to your code and the issue reported from sc_spawn_object is very likely caused via inlining from your Tasker::MethodFunction<> class.  So you will need to look into your code to address these particular reports.

     For SystemC 2.3.1, the are some known Valgrind reports, most of which should have been addressed in SystemC 2.3.3.

    Greetings from Duisburg,
      Philipp

  8. Hi Wim,

    On 11/25/2019 at 5:41 PM, wvandamm said:

    This seemed to imply to me that it is possible to have a cci_param which contains a cci_value_list or cci_value_map with entries that are cci_values and hence have individual handles

    Currently, cci_param<T> indeed does not support any of cci_value, cci_value_list, cci_value_map as value type T. The main reason is, that it would otherwise cause ambiguities in the API of cci_value(_list,_map) itself, if the generic conversion functions (which are used by cci_param internally) would support this.  However, it might be possible to extend cci_param<T> to support this scenario explicitly.

    Greetings from Duisburg,
      Philipp

  9. As the error message says, you have a failing assertion in your testbench code in adder_testbench.cpp, line 30 (emphasis mine):

    20 hours ago, Avnita said:

    simv: /home/avnita/Workspace/SYSTEM_C_FILES/adder_testbench.cpp:30: void testbench::process(): Assertion `COUT == SC_LOGIC_1' failed.

    There is not much we can do about this without knowing your testbench and your model.

    Hope that helps,
       Philipp

  10. Thanks for the report.  I don't fully understand the summary, though.  Some questions:

    Are the changes to the API check and/or the exception handling in sc_simcontext.cpp really needed? I would hope that removing/skipping the assert in sc_cor_qt.cpp is sufficient to work around the mprotect restrictions on CentOS 7+?

    Your instructions do not include --enable-shared=no, but your description says that you only(?) see it on a static SystemC library. Can you please clarify?

    To better understand the failing mprotect call in your environment, can you please provide the value of errno after the call?  This can be obtained by adding something like:

    #include <errno.h>
    // ...
    
    ret = mprotect( ... );
    if( ret != 0 ) {
      int mprotect_errno = errno;
      printf( "mprotect errno: %d, %s\n", mprotect_errno, strerror(mprotect_errno) );
    }

     

    IIRC, Linux generally allows calling mprotect on allocated memory.  The memory needs to be properly aligned at a page boundary, of course.  One option might be to allocate the stack memory via posix_memalign (if available) instead of new.  We can also change the implementation to gracefully ignore a failing protection and only restore the permissions if the mprotect(...,PROT_NONE) call was successful earlier.

    This brings me to my remaining question: Which one of the mprotect calls actually fails: Is it the one removing the protection (PROT_NONE) or the one trying to restore them?

    Thanks,
      Philipp

×
×
  • Create New...