Re: sc_event_and/or_list

John,

Wrt. sc_event_list::swap, this can be implemented quite
straight-forward. Of course, swapping two lists would only be allowed,
if neither of both is currently being waited on. OTOH, this may not be
too important.

I agree, that more complex event expression may be too much effort to
add for now. The most obvious use-cases for this, like user-defined
reset mechanisms e.g.

  wait( block1.finished() & block2.finished() | my_reset );

can now be built on top of the process control extensions.
More over, I think that in case of complex lists you most likely need to
determine, which (subset of) events actually fired.

Thanks,
  Philipp

On 05/10/10 12:27, john.aynsley@doulos.com wrote:
> 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.