Re: sc_event_and/or_list

From: Philipp A. Hartmann <philipp.hartmann@offis.de>
Date: Wed Oct 20 2010 - 10:32:55 PDT

Eric,

the currently proposed API only provides conversions from expression
objects to const list references. So yes, the following hack
intentionally breaks at compile-time:

sc_event e[3];
sc_event_or_list & list1 = e[0] | e[1];

error: invalid initialization of non-const reference of type
‘sc_core::sc_event_or_list&’ from an rvalue of type
‘sc_core::sc_event_or_expr’

I think, this is a Good Thing(tm). The returned lists from expression
objects are currently assumed to be temporaries, which do not need to be
kept around.

Most importantly, due to the now "non-mutating" behaviour of the
operators, existing models exploiting this hack would break anyhow:

 list1 | e[2]; // list1 is NOT extended!

So I don't see a direct way to support these non-standard models.
An implementation could of course add a migration hint to its release
notes to drop the ampersand sign from the variable definition and use
the in-place operators |=,&= for list extension.

Thanks,
  Philipp

On 20/10/10 19:07, Roesler, Eric E wrote:
> John, Philipp,
>
> I have not had a chance to dig into the details of your proposal, but at a high level a better-defined and properly useable set of and/or lists is an improvement for SystemC users (as we agreed at the beginning of the thread).
>
> My thoughts are 1) it is a high priority to make any changes backward compatible w/ 1666-2005, which you say you think they are. Also, for those that abused the old system, it should generate a compiler error, correct, since you are changing the types returned by the & | operators? They should no longer be able to assign those to explicit references to sc_event_and/or_list? I think that is OK - the most important thing is to avoid a runtime failure. Or, could you provide conversion operators that enable conversion to the list objects, but mark them deprecated? This would minimize code changes for end users, and encourage a path to "become legal" :).
>
>
> EricR
>
>
> ________________________________
> From: owner-systemc-p1666-technical@eda.org [mailto:owner-systemc-p1666-technical@eda.org] On Behalf Of john.aynsley@doulos.com
> Sent: Wednesday, October 20, 2010 4:15 AM
> To: Philipp A. Hartmann
> Cc: SystemC P1666 Technical; Jeremiassen, Tor
> Subject: Re: sc_event_and/or_list
>
> Philipp, All,
>
> Given the lack of traffic on the reflector, I am assuming everyone is in agreement with Philipp and I continuing to thrash out this implementation.
>
> I would be okay with saying that If an event list is already destroyed, when a sensitive process is triggered (and therefore needs to access this list), the behaviour shall be undefined.
>
> Note that according to the fine print of the LRM, this gives a smart implementation the choice of detecting and reporting an error.
>
> Now for matching the LRM with the implementation. The wait/next_trigger methods have an argument of type sc_event_and/or_list, which is currently an implementation-defined class name. The proposal is to promote sc_event_and/or_list to standard classes. We then also need to introduce sc_event_and/or_expr classes in order to provide the & | operators used in calls to wait/next_trigger. I propose that sc_event_and/or_expr should become implementation-defined "daggered" names, just as sc_event_and/or_list are at present.
>
> Note that this approach is (I think) backward compatible with 1666-2005, but is not backward compatible with any application code that abused the sc_event_and/or_list classes from the 1666-2005 standard (which forbade their explicit use in an application).
>
> John A
>
>
> From:
>
> "Philipp A. Hartmann" <philipp.hartmann@offis.de>
>
> To:
>
> John Aynsley <john.aynsley@doulos.com>
>
> Cc:
>
> SystemC P1666 Technical <systemc-p1666-technical@eda.org>, "Jeremiassen, Tor" <tor@ti.com>
>
> Date:
>
> 13/10/2010 19:07
>
> Subject:
>
> Re: sc_event_and/or_list
>
>
> ________________________________
>
>
>
> John, All,
>
> John found a problem in the prototypical implementation of a possible
> new sc_event_list interface. The detection of prematurely destroyed
> event lists is not reliably possible in all cases:
>
> If an event list is used as a member of a module, its lifetime may
> correctly end during the destruction of the module hierarchy, although
> there are still child processes of the current module waiting for it.
>
> Detecting (and ignoring) this corner-case is quite difficult for
> similar reasons to the sc_is_running() return value during the
> destruction of the system: The event list is already destroyed before
> the ~sc_module() destructor is called (where process cleanup could occur).
>
> Consequently, my suggestion would be that an implementation should not
> be obliged to detect insufficient lifetimes of passed event lists. If
> an event list is already destroyed, when a sensitive process is
> triggered (and therefore needs to access this list), the behaviour shall
> be undefined.
>
> A workaround to at least detect most prematurely deleted event lists
> would be to perform the check only within running processes. This
> catches at least the "next_trigger( all_events() )" case:
>
> void
> sc_event_list::report_premature_destruction() const
> {
> if( sc_get_current_process_handle().valid() ) {
> sc_assert( false && "sc_event_list prematurely destroyed" );
> }
> }
>
> Malicious users could of course still (e.g. manually) delete event lists
> during the update() phase or between calls to sc_start().
>
> What do you think?
>
> Philipp
>
> On 13/10/10 18:20, Philipp A. Hartmann wrote:
>> As a quick follow-up:
>>
>> Currently, the OSCI-kernel does not delete (or at least kill() remove
>> from internal queues) any child-process of destroyed modules. This is
>> in fact already a problem, since you can easily do something like
>>
>> SC_CTOR( some_module )
>> {
>> sub_module sub("sub") // on stack, instead of member
>> }
>> // sub destroyed too early, processes in "sub" will crash,
>> // since their parent object is lost
>>
>> This has been asked several times in the user-fora.
>> Maybe, we should define this behaviour as well?
>>
>> One soulution could be to remove child processes upon destruction of a
>> module. (what about other child objects?) Child processes are likely to
>> break, when their module ceases to exist anyhow.
>>
>> What do you think?
>> Can we leave this as a pure implementation issue?
>>
>> Thanks,
>> Philipp
>>
>> On 13/10/10 17:40, Philipp A. Hartmann wrote:
>>> John,
>>>
>>> Yes, I've seen this with member-lists, when sc_main exits and objects
>>> (and their member lists) are destroyed, but processes are still waiting
>>> for them. I hoped, that you wouldn't notice ... ;-)
>>>
>>> Something like the following shows the problem:
>>>
>>> SC_MODULE(module)
>>> {
>>> sc_event e;
>>> sc_event_and_list list1;
>>> sc_process_handle h;
>>>
>>> SC_CTOR(module)
>>> : list1( e )
>>> {
>>> SC_THREAD(do_it);
>>> h = sc_get_last_created_process_handle();
>>> }
>>> void do_it()
>>> {
>>> wait( list1 ); // wait forever
>>> }
>>> };
>>>
>>> int sc_main( int, char*[] )
>>> {
>>> module m("m");
>>> sc_start();
>>>
>>> // reset (only) process:
>>> // m.h.reset();
>>>
>>> std::cout << "sc_main exiting..." << std::endl;
>>> return 0;
>>> }
>>>
>>> Do you see this in other cases as well?
>>>
>>> The above corner case looks like a bug in the kernel, which does not
>>> clean up processes properly, though. You can currently ignore it, if it
>>> occurs after the 'sc_main exiting...' message. Local variables in
>>> SC_THREAD processes should not have this problem (since they are not
>>> destroyed).
>>>
>>> Another possibility is to reset() the processes manually, as outlined in
>>> the above code.
>>>
>>> Of course, you can also change sc_event.cpp to do nothing in
>>> sc_event_list::report_premature_deletion().
>>>
>>> Hope that helps,
>>> Philipp
>>>
>>> On 13/10/10 17:24, john.aynsley@doulos.com wrote:
>>>> Philipp,
>>>>
>>>> With the new sc_event.* files, with pretty much any event list
>>>> manipulation, I get:
>>>>
>>>> Fatal: (F4) assertion failed: false && "sc_event_list prematurely
>>>> destroyed"
>>>> In file: ../../../../src/sysc/kernel/sc_event.cpp:479
>>>>
>>>> I simply copied the files on top of the previous patched version 8=(
>>>>
>>>> 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:
>>>> 12/10/2010 18:08
>>>> Subject:
>>>> Re: sc_event_and/or_list
>>>>
>>>>
>>>>
>>>> John,
>>>>
>>>> sorry for the delay. I finally found some time to look at this.
>>>> Comments embedded below.
>>>>
>>>> On 12/10/10 16:22, john.aynsley@doulos.com wrote:
>>>> [snip]
>>>>
>>>>> Using Philipp's prototype we can do some cool stuff with event lists,
>>>> for
>>>>> example:
>>>> [snip some code examples]
>>>>
>>>>> sc_event_and_list list3 = list2 & e2; // Caveat - modifies list2
>>>>> wait( list3 );
>>>>
>>>> I agree, that this is an unexpected side-effect. I would suggest to add
>>>> &= and |= operators to the lists to modify lists in place, though:
>>>>
>>>> class sc_event_or_list {
>>>> // ...
>>>> sc_event_or_list& operator |= ( const sc_event& );
>>>> sc_event_or_list& operator |= ( const sc_event_or_list & );
>>>>
>>>> sc_event_or_expr operator | ( const sc_event& ) const;
>>>> sc_event_or_expr operator | ( const sc_event_or_list& ) const;
>>>> };
>>>>
>>>> (sc_event_or_expr is an implementation defined helper class, as you
>>>> suggested, see below).
>>>>
>>>>> However, because the ctor sc_event_and_list(const sc_event&) is
>>>> protected,
>>>>> we cannot do
>>>>>
>>>>> sc_event_and_list list5 = e3;
>>>>>
>>>>> I think it would be nice to be able to initialize a list with a single
>>>>> event, so I propose we make this ctor public (but without putting the
>>>>> auto_delete argument in the LRM)
>>>>
>>>> I'm not sure, if this is really required. With the '&|='-operators, the
>>>> same could be written as:
>>>>
>>>> sc_event_and_list list5;
>>>> list5 &= e3;
>>>>
>>>> Nevertheless it seems not to cause real harm in the usual case. But
>>>> note, that for your desired syntax these constructors can not be
>>>> 'explicit', which may lead to ambiguities if some functions are
>>>> overloaded for sc_event_and_list and sc_event_or_list but not sc_event
>>>> (not in the kernel, though).
>>>>
>>>>> Also, we cannot do
>>>>>
>>>>> sc_event_and_list list6 = list3 & list4;
>>>>>
>>>>> because we have no sc_event_and_list& operator & ( const
>>>>> sc_event_and_list& );
>>>>> I propose we add this operator
>>>>
>>>> Indeed, this would be very useful. Implemented in the attached quick'n
>>>> dirty prototype as well.
>>>>
>>>>> Finally, note that
>>>>>
>>>>> list2 = list2 & e1;
>>>>>
>>>>> is equivalent to
>>>>>
>>>>> list2 & e1;
>>>>>
>>>>> the latter being permitted because operator& works through a
>>>> side-effect.
>>>>> Would this be a reason for introducing a separate class for
>>>>> user-instantiated event lists?
>>>>
>>>> Yes, this is a strong reason for adding implementation-defined
>>>> expression helper classes. They can be implemented fairly easily around
>>>> a temporary (auto-deleted) sc_event_list, which gives up ownership upon
>>>> conversion to a list object.
>>>>
>>>> I have attached updated sc_event.(h,cpp) classes, which implement nearly
>>>> all of the proposed extensions:
>>>>
>>>> - construction from single events
>>>> - expression classes (sc_event_and_expr, sc_event_or_expr)
>>>> - list concatenation ( list1 & list2 )
>>>> - operator &=, |=
>>>> - size()
>>>> - swap()
>>>> - move-semantics for temporary lists (implementation detail)
>>>>
>>>> The implementation is backwards-compatible, even regarding the
>>>> 'reference hack' and can detect premature deletion (or modification) of
>>>> busy event lists (i.e. with currently sensitive processes).
>>>>
>>>> The new classes should work with the earlier proposed 'const patch',
>>>> where wait() and next_trigger() take const list-references. This
>>>> earlier patch (against OSCI PoC version 2.3.10dec09_beta) can be found
>>>> in the Twiki [1].
>>>>
>>>> Greetings from Oldenburg,
>>>> Philipp
>>>>
>>>> [1]
>>>> http://www.eda.org/twiki/pub/Main/PhilippHartmann/0001-make-event-handling-const.patch
>>>>
>>>>
>>>
>>>
>>
>>
>
>
> --
> 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/
> [attachment "sc_event.cpp" deleted by John Aynsley/doulos]
>
>
> --
> This message has been scanned for viruses and
> dangerous content by MailScanner<http://www.mailscanner.info/>, and is
> believed to be clean.
>

-- 
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.
Received on Wed Oct 20 10:33:43 2010

This archive was generated by hypermail 2.1.8 : Wed Oct 20 2010 - 10:33:46 PDT