Re: Ambiguities in certain combinations of SystemC sc_int/sc_fixed datatypes

All,

I have made the 7 convert-from-built-in constructors explicit in the two classes sc_fxval and sc_fxval_fast.

Please let me know if you need further changes.

John A

-----"Philipp A. Hartmann" <philipp.hartmann@offis.de> wrote: -----
To: john.aynsley@doulos.com
From: "Philipp A. Hartmann" <philipp.hartmann@offis.de>
Date: 12/03/2010 02:01PM
Cc: acg@forteds.com, bpriya@cadence.com, david.long@doulos.com, Michael McNamara <mcnamara@cadence.com>, P1666 Technical WG <systemc-p1666-technical@eda.org>
Subject: Re: Ambiguities in certain combinations of SystemC sc_int/sc_fixed datatypes

John, Andy, All,

sorry for the delayed reply. Find a summary of changes and a response
to Andy's comments below.

Yes, John, the only proposed changes to the LRM would be to make the
constructors of the classes sc_fxval and sc_fxval_fast explicit for the
following parameter overloads:

  int
  unsigned int
  long
  unsigned long
  float
  double
  const char*

  Potentially add a note, that (for backwards compatibility) an
implementation can add honour a #define SC_FXVAL_IMPLICIT_CONV to keep
the old behaviour. This has been suggested by Dave and is certainly a
good idea. I'm not sure, whether this needs to be included in the LRM,
though.

Secondly, Andy had a comment on the issue off-list:

On 02/12/10 17:08, Andy Goodrich wrote:
> another possibility would be to make the constructors guarded by
> SC_FX_EXCLUDE_OTHER explicit. Those are the ones that are causing the
> issue I believe. If you define SC_INCLUDE_FX and SC_FX_EXCLUDE_OTHER
> then the problem does not occur. The question is, which do we want to
> be explicit?

NB: SC_FX_EXCLUDE_OTHER is a define that is honoured by the ref-sim to
    exclude the conversion constructors from sc_uint et.al. completely
    from sc_fxval.

As I see the case, Andy is right for the initial ambiguity that has been
raised by Bishnupriya:

  {
    sc_uint<8> A,B;
    sc_uint<8> C,D;
    A = B + C * D; // works with SC_FX_EXCLUDE_OTHER
  }

Unfortunately, with this define mixed integer/fixed-point expressions
then break with a similar ambiguity:

  {
    sc_fixed<8,2> A,B;
    sc_uint<8> C,D;
    A = B + C * D; // fails with SC_FX_EXCLUDE_OTHER
  }

  AFAICS, the constructors guarded by SC_FX_EXCLUDE_OTHER are already
explicit (in the LRM and in the ref-sim). Changing these to be implicit
as well does not help fixing things.

  Consequently, I would still propose to implement the above change to
make the remaining "from-builtin" sc_fxval constructors explicit, with
the drawback that a minor backwards compatibility issue is introduced,
regarding copy-initialisation:

  sc_fxval v = 10;

Such definitions have to be changed to one of the following variants:

  sc_fxval v(10); // direct initialisation
  sc_fxval v = sc_fxval(10); // explicit cast
  sc_fxval v; v = 10; // separate construction + assignment

With an additional preprocessor directive to enable the old behaviour, I
think the benefits are definitely outweighing the cost here.

Thoughts?

Greetings from Oldenburg,
  Philipp

On 03/12/10 05:43, john.aynsley@doulos.com wrote:
> Philipp,
>
> Could you please summarize for me the LRM changes needed here? Is it literally just two "explicit"s?
>
> Thanks,
>
> John A
>
> -----"Philipp A. Hartmann" <philipp.hartmann@offis.de> wrote: -----
> To: Andy Goodrich <acg@forteds.com>
> From: "Philipp A. Hartmann" <philipp.hartmann@offis.de>
> Date: 12/01/2010 10:23PM
> Cc: John Aynsley <john.aynsley@doulos.com>, Bishnupriya Bhattacharya <bpriya@cadence.com>, David Long <david.long@doulos.com>, "Michael (Mac) McNamara" <mcnamara@cadence.com>, SystemC P1666 Technical <systemc-p1666-technical@eda.org>
> Subject: Ambiguities in certain combinations of SystemC sc_int/sc_fixed datatypes
>
> Andy,
>
> on the P1666 technical reflector [1], Bishnupriya brought up a quite
> long-standing issue related to unwanted operator ambiguities for
> sc_(u)int(_base), that can arise when enabling the fixed-point types via
> SC_INCLUDE_FX.
>
> As one of the possible solutions, I have then suggested to make the
> conversion constructors from the builtin types of sc_fxval(_fast)
> explicit, since seems to fix the issue (with some minor backwards
> compatibility limitations), see [2] for a summary.
>
> There has been a side discussion with Mac, that there may be other
> cases of ambiguities (longer expressions, purely containing sc_fixed),
> which I could not reproduce with the reference simulator.
>
> Attached is an archive with a patch against the latest OSCI internal
> version of the reference implementation providing the proposed change.
> Would you please have a look at this issue and the proposed solution?
> I'm afraid, that we might be missing some subtle corner case here...
>
> Thanks,
> Philipp
>
> 1.
> http://www.eda.org/systemc/systemc-p1666/systemc-p1666-technical/hm/0815.html
> 2.
> http://www.eda.org/systemc/systemc-p1666/systemc-p1666-technical/hm/0828.html
>
> Contents of attached archive:
>
> sc_fxval_test.cpp
>
> quick test for some of the problematic expressions
>
> 0001-sc_fxval-_fast-make-ctors-explicit.patch
>
> patch to make some of the constructors of sc_fxval(_fast)
> explicit, unless SC_FXVAL_IMPLICIT_CONV is defined
> (for backwards compatibility, as suggested by David).
>
> 0001-fix-tests-for-sc_fxval-explicit-constructors.patch
>
> patch against the SystemC regression tests to fix the
> backwards compatibility issues in some of the fixed-point
> tests
>
> 0002-sc_dt-div_scfx_rep-fix-default-max-word-length.patch
>
> patch, fixing a bug in SystemC 2.3.09jul10_beta, where
> the default argument of the sc_dt::div_scfx_rep function
> has been broken (unrelated)
>

-- 
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 ú http://offis.de/en/
Phone/Fax: +49-441-9722-420/282 ú PGP: 0x9161A5C0 ú Skype: phi.har
 
-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.