Re: sc_event_and/or_list

Philipp, Tor, All,

For those of you who are not up-to-speed with the issues, I will try to
summarize.

Consider the following code fragment:

sc_event e1, e2;
...
wait( e1 | e2 );

What happens is that sc_event::operator| creates an sc_event_or_list on
the heap that is marked for auto-deletion. When the process that called
wait resumes following the notification of e1 or e2, the sc_event_or_list
object is deleted (because it was marked for auto-deletion).

However, the protected constructor

sc_event_or_list( const sc_event&, bool auto_delete_ = false );

does have an (unused) mechanism to construct an event list that is not
marked for auto-deletion. We could harness this mechanism to allow
applications to create proper event list objects. It would then be the
responsibility of the application to delete the event list object. As
Philipp points out in his example, the application has to ensure that the
event list object does not get deleted prematurely, for example by being a
stack variable in a method process.

Now for my opinion on Philipp's questions below:

I think it is reasonable to require an application to keep an explicitly
constructed event list object around until the process has resumed, and
for the implementation to have the option of detecting and reporting the
error otherwise. I would be somewhat opposed to introducing the overhead
of having wait/next_trigger copy the event list.

In my opinion, the mutation case (c) should be outlawed. operator| was
originally protected, and not intended as an application-level side-effect
operator.

I'm not sure we can disallow temporaries altogether, because the code
fragment

wait( all_events() );

should be allowed in my opinion, which I think means that we need to make
the const patch as proposed by Philipp. Does that make sense?

Thanks,

John A

From:
"Philipp A. Hartmann" <philipp.hartmann@offis.de>
To:
john.aynsley@doulos.com
Cc:
systemc-p1666-technical@eda.org, "Jeremiassen, Tor" <tor@ti.com>
Date:
30/09/2010 14:54
Subject:
Re: sc_event_and/or_list

John,

regarding the implementation of 'const'ness of sc_event_lists please
find source code and comments below.

But actually, I'm not sure about the correct semantics anyhow. During
my experiments, I've realised, that we still have to define the correct
ownership and lifetime requirements for the event lists.

Consider the following example:

SC_METHOD(proc);
// ...

sc_event_and_list all_events(); // return temporary

void proc()
{
  if( case_1 )
    // (a) wait on temporary list
    next_trigger( all_events() );
  else {
    // (b) wait on local list
    sc_event_and_list el = all_events(); // copy list
    next_trigger( el ); // would even work for sc_event_list& arg

    // (c) change list, what now?
    el | some_event; // relation to previous next_trigger call?
  }
}

In both cases (a) and (b), the event lists are destroyed _before_ the
kernel had a chance to complete its processing on it.

I see two possible solutions for this lifetime problem:

 1) the kernel makes a copy of the list internally
    -> copies come at some cost, but temporaries are possible now
 2) the kernel detects the premature deletion of the list and
    errors out
    -> Then we don't need to change the signatures of wait() and
       next_trigger(), since temporaries are nearly always an user
       error.

For mutating event lists (case (c) in example above), only taking copies
seems to be viable. We could of course prohibit mutations, when the
kernel "uses" the event list currently.

Opinions on that?

Constness discussion interspersed below.

On 30/09/10 10:45, john.aynsley@doulos.com wrote:
[snip]
> I hit the following roadblock in file sc_thread_process.h:
>
> inline
> void
> sc_thread_process::wait( const sc_event_or_list& el ) // <<<< Changed
> code
> {
> el.add_dynamic( this );

The add_dynamic() member function (and friends) of sc_event itself are
already const. Changing these functions to be const in sc_event_list
does not do much harm:

 void
-sc_event_list::add_dynamic( sc_thread_handle thread_h )
+sc_event_list::add_dynamic( sc_thread_handle thread_h ) const
 {
     if ( m_events.size() != 0 ) {
- const sc_event** l_events = &m_events[0];
+ const sc_event* const * l_events = &m_events[0];
       for( int i = m_events.size() - 1; i >= 0; -- i ) {
           l_events[i]->add_dynamic( thread_h );
       }

[snip]
>
> In other words, it seems dynamic sensitivity does change the event
lists.
> Did you get any further than this? Have you got an answer?

Well, yes. The event lists are not really changed in the sense, that
the set of represented events changes. There are currently 'non-const'
functions called, which in their body only call const functions on the
events.

  This is not really pretty, but adding/removing sensitive processes
to/from events has to be a "const" operation, since wait() only gets
const event references. So we don't make anything worse here.

I've uploaded a 'const-adding' patch against SystemC 2.2.1 (or 2.2.0) to
my Twiki-Space at eda.org [1].

Thanks,
  Philipp

[1]
http://www.eda.org/twiki/pub/Main/PhilippHartmann/0001-make-event-handling-const.patch

>
> From:
> "Philipp A. Hartmann" <philipp.hartmann@offis.de>
> To:
> john.aynsley@doulos.com
> Cc:
> "Jeremiassen, Tor" <tor@ti.com>, systemc-p1666-technical@eda.org
> Date:
> 28/09/2010 22:08
> Subject:
> Re: sc_event_and/or_list
>
>
>
> Ok, I had a misconception in my last mail. Actually, dynamic
> sensitivity does not change the event lists (in terms of represented
> events) in the current OSCI implementation. This removes my main
> concern mentioned in the last mail. Sorry for that.
>
> So the current situation is as follows:
>
> - changing wait()/next_trigger() to take "const sc_event_*_list &"
> works fine (only some additional 'const' qualifiers required,
> no additional 'mutable' or 'const_cast').
> -> would enable temporary sc_event_*_lists as parameters
>
> - For efficiency reasons, the operators &,| for building
> event lists could keep their mutating behaviour.
>
> - Wrt to the public interface of sc_event_*_list objects, I have
> no strong opinion. Except that obvious things like begin(), end(),
> size(), swap() should probably be part of it. :-)
>
> John, if you're interested in my experimental code, please let me know.
>
> Thanks,
> Philipp
>
> On 28/09/10 22:18, Philipp A. Hartmann wrote:
>> John, Tor, All,
>>
>> from my POV, the most important issue in this regard is the definition
>> of value/ownership semantics of such lists in their interaction with
the
>> simulation kernel.
>>
>> As of know, the implementation is completely free to modify the
passed
>> event lists (as the interface suggests). Having these lists as proper
>> objects (e.g. module members) would imply, that the kernel should NOT
>> modify the passed objects, right?
>>
>> Changing the signatures of wait() and next_trigger to take event
lists
>> as const references would be possible and even backwards-compatible
(and
>> would solve the return-by-value issue, John mentioned).
>>
>> Example:
>>
>> sc_event_and_list el1 = e1 & e2 & e3;
>> sc_assert( el1.size() == 3 ); // 3 events in list
>>
>> // should el1 be unchanged ? (IMHO, yes)
>> sc_event_and_list el2 = el1 & e4;
>> sc_assert( el1.size() == 3 && el2.size() == 4 ); // (1)
>>
>> // should el1 be unchanged after wait? (IMHO, yes)
>> wait( el1 );
>> sc_assert( el1.size() == 3 ); // (2)
>>
>> With the current OSCI PoC simulator, the assertions (1) and (2) would
>> NOT hold.
>>
>> On the other hand, changing operators &,| and the
>> wait()/next_trigger() functions to make an explicit copy internally
>> comes with quite some runtime overhead: In a quick and dirty test I've
>> measured up to +30% for naïve enforcing (1)+(2) and +7% for enforcing
>> (2) only.
>>
>> Opinions?
>>
>> Greetings from Oldenburg,
>> Philipp
>>
>> On 28/09/10 13:41, john.aynsley@doulos.com wrote:
>>> Tor, All,
>>>
>>> I think we have a consensus that we want to make the change such that
>>> sc_event_and/or_list can be instantiated as proper objects and act as
>>> containers for events that can be passed to wait() and next_trigger()
> (at
>>> least).
>>>
>>> Would you (or someone else) please make a specific proposal for
classes
>
>>> sc_event_and_list and sc_event_or_list to go into the LRM. I started
>>> prototyping this months back and ran into some C++ issues. As I recall

>>> from my experiments, the prototype wait(sc_core::sc_event_or_list&)
>>> currently precludes having something like wait(all_events()); where
>>> all_events() returns an event list by value. So we need a set of
>>> signatures for the constructor(s)/operators of the event list classes
> that
>>> work with the current signatures of wait/next_trigger.
>>>
>>> John A
>>
>
>

-- 
Philipp A. Hartmann
Hardware/Software Design Methodology Group
OFFIS Institute for Information Technology
R&D Division Transportation · FuE-Bereich Verkehr
Escherweg 2 · 26121 Oldenburg · Germany
Phone/Fax: +49-441-9722-420/282 · PGP: 0x9161A5C0 · http://www.offis.de/
-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.