Re: sc_event_and/or_list

Philipp,

My opinions below...

John A

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:
04/10/2010 19:17
Subject:
Re: sc_event_and/or_list

John, Tor, All,

I completely agree with your analysis. Moreover, I did some experiments
regarding the tracking of prematurely destroyed (or modified) event
lists, which seems to work quite well.

  I've added the required kernel changes (against OSCI-internal SystemC
2.3.16jul10_beta) to my Twiki page again (two patches [1],[2] and "new"
sc_event.h [3] and sc_event.cpp [4]).

  Note, that currently an application can only default-create
non-auto-deleted sc_event_*_lists or copy/assign from an (e1 |,& e2)
expression. I think, the auto-delete mechanism should be hidden from an
application. Otherwise, it's next to impossible to get the lifetimes
correctly implemented in presence of copyable event lists which are
required for return-by-value as in all_events().

*** [JA] I agree the auto-delete mechanism should be hidden from the
application

  Premature deletion and modifications while processes are still waiting
for an event list can be detected (see source). Temporaries are
supported, when using the const patch as proposed earlier.

Some questions remain:

  - Should the lists have any additional public interface?
    Maybe a size() function would be useful? Maybe swap()?

*** [JA] I agree with size(). I don't know whether adding swap() causes
implementation issues.

  - Should empty lists trigger an error or a warning, when being passed
    to wait()/next_trigger? Or should it be silently ignored?
    wait( timeout, sc_event_*_list ) might be useful even for empty
    lists.

*** [JA] I would not want to change the rules for long-standing constructs
like this. Hence I am tending toward saying that an empty event list is an
ERROR

  - What about sc_event_list (the base class) itself? Should an
    application be able to use it explicitly? I think, it could
    be left out of the standard as being an implementation artefact.

*** [JA] I vote for implementation artefact.
*** [JA] As an aside, I often hear people ask whether they can use
combinations of and/or operators in an event list, as in

wait( ((e1 & e2) | e3) & e4) );

*** [JA] I kind of assume we don't want to pick that one up ;-)

Thanks,
  Philipp

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

[2]
http://www.eda.org/twiki/pub/Main/PhilippHartmann/0002-sc_event_list-track-in-kernel-usage.patch

[3] http://www.eda.org/twiki/pub/Main/PhilippHartmann/sc_event.h
[4] http://www.eda.org/twiki/pub/Main/PhilippHartmann/sc_event.cpp

On 04/10/10 17:15, john.aynsley@doulos.com wrote:
> 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.