Re: Multiple writers to sc_signal

Philipp,

Thanks for doing the experiments. Excellent!

Having get_writer_policy() would be very nice, so I vote we add the
virtual method to class sc_signal_write_if. As you said, this would not
be pure virtual but would return the value "not OK" for backward
compatibility

John A

From:
"Philipp A. Hartmann" <philipp.hartmann@offis.de>
To:
john.aynsley@doulos.com
Cc:
P1666 Technical WG <systemc-p1666-technical@eda.org>
Date:
10/11/2010 22:33
Subject:
Re: Multiple writers to sc_signal

John,

I did some experiments with both, constructor and the template-based
approach. In the checked case, the constructor-based implementation
adds a small, but measurable overhead compared to the current non-policy
case -- up to 5% in a contrived test that merely consists of repeatedly
writing to a signal.

  So I agree, that we should go for the template approach, which even
speeds up the unchecked case by about the same amount.

  But, a minor catch came to light wrt to the get_writer_policy()
function: This can't be implemented in a useful way, since there is no
policy agnostic base-class, a user model could query. And calling
get_writer_policy() after an explicit cast to
sc_signal<T,SC_SIGNAL_MULTIPLE_WRITERS_OK>* is kind of moot, I think. ;-)

  If we want this feature, we would need to include it as a virtual
function e.g. in sc_signal_write_if<T> (or inout_if), probably already
implemented as returning SC_SIGNAL_MULTIPLE_WRITERS_NOT_OK by default
for backwards compatibility. The signal could the override this
function with the real policy.

Greetings from Oldenburg,
Philipp

NB: My extended policies showed up to be easily implemented without
    overhead compared to the NOT_OK case, and even providing some
    benefits depending on the use case. But this seems to be something
    for either implementation-defined extensions or some future version
    of IEEE 1666.

On 10/11/10 12:11, john.aynsley@doulos.com wrote:
> Philipp, All,
>
> I can see the sense in Philipp's proposal to have several distinct
> policies to allow/disallow multiple writes in the same and in separate
> deltas, but based on the premise that people want a simple solution, I
am
> keeping to my position of just two choices, effectively formalizing in
the
> LRM the two ways (legal and illegal) that people have used sc_signal in
> the past. Please speak up, everybody!
>
> I guess the pure approach is the use a template parameter, as Stuart
> proposed, to maximize speed for the unchecked case.
>
> I agree that a specialized port in a piece of IP can do an
> end_of_elaboration check that it is indeed bound to a promiscuous
> sc_signal.
>
> John A
>
>
>
>
> From:
> "Philipp A. Hartmann" <philipp.hartmann@offis.de>
> To:
> john.aynsley@doulos.com
> Cc:
> P1666 Technical WG <systemc-p1666-technical@eda.org>
> Date:
> 10/11/2010 10:48
> Subject:
> Re: Multiple writers to sc_signal
>
>
>
> John, all,
>
> answers below.
>
> On 10/11/10 10:59, john.aynsley@doulos.com wrote:
>>
>> If we were starting from a clean sheet, there may be merit in having
>> distinct policies allowing or disallowing signal writes within the same

>> delta. But simulation-time multiple writer checks are expensive, which
> is
>> why the PoC simulator turned them off by default. So I am only in
favour
>
>> of two policies, as originally proposed by Stuart:
>
> As it stands now, the PoC simulator actually _does_ checking for
> multiple writers by default, even on every write() call unless the
> environment variable SC_SIGNAL_WRITE_CHECK is set to DISABLE.
>
> This is done by storing the sc_object * of the writing process and
> compare it with the current one, if it's already set. So allowing for
> multiple writers only in distinct deltas merely boils down to forgetting
> the writing process during the update() phase. Why should this be
costly?
>
>> * SC_MULTIPLE_WRITERS_NOT_OK = the default, the implementation should
>> check for multiple writers at every level and issue error reports
>
> This is already the default, yes.
>
>> * SC_MULTIPLE_WRITERS_OK = no checks, period.
>> What do people think?
>
> As said above, an SC_MULTIPLE_WRITERS_RELAXED is more or less the
> addition of the following lines to the implementation:
>
> if( m_policy & SC_MULTIPLE_WRITERS_RELAXED )
> m_writer = 0;
>
>> I take your point that ports bind to interfaces, not channels, so we
> have
>> the choice of the policy being a template parameter or a constructor
> arg.
>> What do people think?
>
> If you think that performance is a strong issue, a template parameter
> may help the compiler to avoid the checks completely at runtime. But I
> think it should not make any measurable difference.
>
>> Now for binding multiple sc_inout ports to a signal that allows
multiple
>
>> writers. You suggest this is still an error. I suggest this is allowed,

>> but I can see the sense in your argument. What do people think?
>
> I especially want a separation of concerns here. The port check is done
> in register_port() of the signal. It does not affect the dynamic checks
> at all. Since there may not be a use case for a single process writing
> via multiple ports, I would prefer to combine this with the RELAXED
> policy, though.
>
>> Regarding multiple writers to a single sc_inout port, I don't see how
> you
>> are going to detect that at end_of_elaboration, the point being that
you
>
>
> You are right. I don't think, that it is possible to detect multiple
> writers via a single port, since you can't determine from within
> operator->, whether the caller will perform a read or a write.
>
> But, the issue at hand in the original Cadence proposal was to enable
> a way to detect from an output port, whether it will be OK to write to
> this port from multiple processes (within an IP model for instance). If
> you ask the bound channel about its policy, you can determine this at
> end_of_elaboration. Am I missing something?
>
>> don't know whether a process instance is going to write to a signal
> until
>> it does so - during simulation. Hence the need for expensive dynamic
>> checks.
>
> Yes, expensive in terms of a policy query and a pointer comparison. ;-)
> Are there actually any benchmarks on this?
>
>> I am saying that having multiple writers to an sc_inout port is
>> necessarily allowed (because it is undetectable), It can only be
> detected
>
> Yes. But let's keep the deltas distinct by default in that case, as
> proposed in my last mail. A completely unchecked SC_MULTIPLE_WRITERS_OK
> depends on the internal scheduler, is unreliable and can hide errors,
> that are otherwise detected. This should be avoided in models excepts
> for very strong reasons.
>
> I'm really in favour to add a relaxed policy, that keeps the model
> semantics well-defined and solves problems for writing transactors or
> using different dynamic processes. It's trivially implementable (see
> above) and does not impose higher cost than the default policy.
>
>> when there is a signal instance in the loop. I am tentatively proposing

>> that the rule should then be very simple: the signal either allows
>> multiple writers or does not allow multiple writers, irrespective of
how
>
>> many sc_inout ports are involved.
>
> Well, yes. Obviously allowing multiple ports would include allowing
> multiple drivers (in different deltas), see above.
>
> Greetings from Oldenburg,
> Philipp
>
>> In other words, the situation where
>> multiple processes drive the signal through a single sc_inout port is
>> indistinguishable from the situation where multiple processes drive the

>> signal through an equal number of sc_inout ports, one-per-driver. (This

>> scenario does not stop people from creating specialized ports that can
>> only be bound to signals of a particular flavour, if they wish,
>> interrogating the signal using get_writer_policy().) What do people
>> think?
>>
>> John A
>>
>>
>>
>> From:
>> "Philipp A. Hartmann" <philipp.hartmann@offis.de>
>> To:
>> john.aynsley@doulos.com
>> Cc:
>> P1666 Technical WG <systemc-p1666-technical@eda.org>
>> Date:
>> 10/11/2010 00:34
>> Subject:
>> Re: Multiple writers to sc_signal
>>
>>
>>
>> John, All,
>>
>> some points from my side on this topic.
>>
>> On 09/11/10 14:46, john.aynsley@doulos.com wrote:
>>>
>>> I suppose we could just add the policy to sc_signal as proposed by
>> Stuart.
>>> The default would be the current rule, i.e. it is an error for
multiple
>
>>> process instances to write to the same signal, although multiple
writes
>
>>> are permitted during elaboration/sc_main/callbacks. The alternative
>> policy
>>> would remove all restrictions. Is that what everyone means by simple?
>>
>> Hmmm, I'm not sure about removing all restrictions. I would vote to
>> lift only the writing process limitation towards allowing multiple
>> writers at different deltas/points in time. IMHO, this covers the
needs
>> in most (all?) valid use cases and keeps the semantics well-defined.
>>
>> What about the following policies (names to be discussed):
>>
>> enum sc_signal_writer_policy
>> {
>> SC_SIGNAL_WRITER_UNIQUE // default, one port, one process
>> , SC_SIGNAL_WRITER_RELAXED // multiple processes, distinct
deltas
>> , SC_SIGNAL_WRITER_MULTIPLE_PORTS // multiple ports, distinct deltas
>> , SC_SIGNAL_WRITER_UNCHECKED // no restrictions - impl. defined
>> };
>>
>> For the fully unchecked policy, there should be a prominent warning
>> that the actual "winner" of the conflicting writes is (at most)
>> implementation-defined.
>>
>>> I guess that using a template parameter is going to mess up C++ type
>>> compatibility when binding ports and signals. Would we do better to
set
>
>>> the multiple writers policy using a constructor argument (rather than
>>> introducing a non-templated sc_signal_base and all that jazz).
>>
>> AFAICS, we don't need to change anything in the ports, if the policy
>> template parameter is only added to the signal, not to the interfaces.
>> But I agree, that a constructor parameter is sufficient and probably
>> simpler.
>>
>> NB: Wrt to sc_vector, this is a nice example for a simple creator
>> function:
>>
>> template< typename T, sc_signal_writer_policy POL >
>> sc_signal<T>* create_signal( const char * nm, size_t )
>> { return new sc_signal<T>( nm, POL ); }
>>
>> sc_vector< sc_signal<int> > sig_vec;
>> sig_vec.init( 4, create_signal< int, SC_SIGNAL_WRITER_RELAXED > );
>>
>> We should then add an accessor function for the policy, in addition to
>> the constructor:
>>
>> explicit
>> sc_signal( const char* nm,
>> sc_signal_writer_policy pol = SC_SIGNAL_WRITER_UNIQUE );
>>
>> sc_signal_writer_policy writer_policy() const;
>>
>> This way, special ports (or IP vendors) can check the policy at
>> end_of_elaboration() as done in sc_in_resolved and friends.
>>
>> That said, the sc_signal_rv/resolved classes should then of course
>> return SC_SIGNAL_WRITER_UNCHECKED as their policy for consistency, and
>> sc_buffer should get a constructor with a policy as well.
>>
>>> Then what about the semantics of sc_inout? Currently it is an error to

>>> bind more than one sc_inout port to a given sc_signal. Should that
> check
>>
>>> be relaxed depending on the policy of the sc_signal instance?
>>
>> I would like to separate this (see above). IMHO, multiple driving
>> ports indicate a design error in most cases. So let's separate this
>> from the other, safer cases.
>>
>>> What would we say for the case of having multiple writers to an
> sc_inout
>>
>>> port? It is the responsibility of the model writer to highlight the
> need
>>
>>> for the multiple writer policy in their documentation?
>>
>> With the accessor as described above, the model writer could issue an
>> appropriate error at end_of_elaboration(). So I don't see an immediate
>> need for specialised ports in the core language.
>>
>> Greetings from Oldenburg,
>> Philipp
>>
>
>

-- 
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.