Re: ambiguous operators with SC_INCLUDE_FX

Bishnupriya,

yes, your example compiles an works as expected with the proposed patch.
 The ambiguities around sc_(u)int(_base) are the motivation for this change.

The patch essentially adds the 'explicit' keyword to the constructors of
sc_fxval(_fast) for the builtin types:

  int
  long
  unsigned long
  float (new in 1666-2011)
  double
  const char*

With the patch, all existing code should still be working as expected,
and ambiguities with sc_(u)int_base and builtin types are removed.

Only copy-initialisation from builtin types fails now:

  sc_fxval v1 = 1; // not ok, copy initialisation
  sc_fxval v2( 1 ); // ok, direct initialisation
  sc_fxval v3 = (sc_fxval) 1; // ok, copy initialisation with cast
  sc_fxval v4; v4 = 1; // ok, assignment

I tend to agree with Dave, that this is an acceptable limitation.
Opinions?

Greetings from Oldenburg,
Philipp

On 27/11/10 13:31, Bishnupriya Bhattacharya wrote:
> Philip,
>
> The actual issue (of which I presented a simplified form) is the code snippet below. This currently produces the same compile error. Should also get fixed by the proposed fix in option 1 - explicit ctor for converting from native data type to sc_fxval(_fast). Can you confirm?
>
> Thanks,
> -Bishnupriya
>
> #define SC_INCLUDE_FX
> #include <systemc.h>
>
> int sc_main(int argc, char * argv[])
> {
> sc_int<20> a,b,c,d,e,f;
> sc_int<20> val;
>
> #ifdef WORK_AROUND
> val = a + (sc_int<WDTH>)(b * (sc_int<WDTH>)((c * d) + (e * f)));
> #else // BUG ?
> val = a + (b * ((c * d) + (e * f)));
> #endif
>
> sc_start(25);
> return 0;
> }
>
> -----Original Message-----
> From: Philipp A. Hartmann [mailto:philipp.hartmann@offis.de]
> Sent: Saturday, November 27, 2010 5:20 AM
> To: Michael (Mac) McNamara
> Cc: Bishnupriya Bhattacharya; 'systemc-p1666-technical@eda.org'; 'david.long@doulos.com'
> Subject: Re: ambiguous operators with SC_INCLUDE_FX
>
> Sorry, I have misread your original mail.
>
> Unfortunately, I cannot confirm your issue here. The following compiles fine here (with and without the patch under discussion):
>
> #define SC_INCLUDE_FX
> #include <systemc.h>
> int sc_main(int,char*[])
> {
> sc_fixed<8,2> A,B,C,D;
> A = B + C * D;
> sc_fxval t = C * D;
> A = B + t;
> return 0;
> }
>
> Maybe you can look up the exact issue you were experiencing? There could be other causes of ambiguities lurking ...
>
> Greetings from Oldenburg,
> Philipp
>
> On 27/11/10 00:22, Michael (Mac) McNamara wrote:
>> Well today, what I typed will not compile, due to the sc_fxval/sc_fxval_fast issue.
>>
>> So if we add the constructors, this will compile, correct?
>>
>> On that day I will then get essentially
>>
>> sc_fixed A,B,C,D;
>> long long t;
>>
>> t = C * D;
>> A = B + t;
>>
>> which will as you hint will be more accurate than if I had just 8 bit storage for the temporary(s). I can always declare the temporarys to constrain this; or not declare it to get ease of coding, and more precision.
>>
>> Mac
>>
>> ----- Original Message -----
>> From: Philipp A. Hartmann <philipp.hartmann@offis.de>
>> To: Michael (Mac) McNamara
>> Cc: Bishnupriya Bhattacharya; 'systemc-p1666-technical@eda.org'
>> <systemc-p1666-technical@eda.org>; 'david.long@doulos.com'
>> <david.long@doulos.com>
>> Sent: Fri Nov 26 13:20:44 2010
>> Subject: Re: ambiguous operators with SC_INCLUDE_FX
>>
>> Mac,
>>
>> AFAICS, this is a separate issue.
>>
>> The SystemC datatypes all perform (within certain limits)
>> numerically accurate computations of sub-expressions. If you want to
>> explicitly express the loss of precision in intermediate steps, you
>> have to strip these extra bits manually as you describe.
>>
>> I don't see a nice way solution to this, since in the opposite case
>> the situation would be tedious as well, if precision would be lost in
>> (sub-)expressions already:
>>
>> sc_fixed<8,2> A, B;
>> // ...
>> sc_fixed<16,4> C = A * B;
>>
>> Here, C should contain the full result, shouldn't it? Requiring to
>> enlarge (one of) the operands before doing the multiplication suffers
>> from the same maintenance burden, I suppose.
>>
>> Using a meaningful set of typedefs and ?= operators where
>> appropriate can reduce the impact of the datatype's computational
>> semantics during accuracy/datatype exploration.
>>
>> Greetings from Oldenburg,
>> Philipp
>>
>> On 26/11/10 21:02, Michael (Mac) McNamara wrote:
>>> Philipp/David - will this also handle the original situation where I observed this issue (typing from memory) where I had:
>>>
>>> sc_fixed<8,2,...> A,B,C,D;
>>>
>>> ...
>>>
>>> A = B + C * D;
>>>
>>> Today you have to code the math as
>>>
>>> A = B + (sc_fixed<8,2,...>)(C*D);
>>>
>>> or
>>>
>>> A = C*D;
>>> A = B+A;
>>>
>>> which is rather silly, especially if you are playing with data types and modes of a large numerically intensive program to see their effect on precision - you have to insert casts, and change the declaration and the casts throughout the program, or break your program into a series of simple expressions, both which are error prone and ugly and a bit embarrassing...
>>>
>>>
>>> Mac
>>>
>>> ----- Original Message -----
>>> From: owner-systemc-p1666-technical@eda.org
>>> <owner-systemc-p1666-technical@eda.org>
>>> To: Bishnupriya Bhattacharya
>>> Cc: systemc-p1666-technical@eda.org
>>> <systemc-p1666-technical@eda.org>; David Long <david.long@doulos.com>
>>> Sent: Fri Nov 26 03:48:32 2010
>>> Subject: Re: ambiguous operators with SC_INCLUDE_FX
>>>
>>> Bishnupriya,
>>>
>>> to me, the root cause of the ambiguity issue is the implicit
>>> conversion from the native data-types to sc_fxval(_fast). The
>>> constructors are not 'explicit', therefore these classes get in our
>>> way in this case. But for the sake of backwards compatibility, we
>>> can't change sc_fxval(_fast), I suppose.
>>>
>>> So we have to add operators to sc_(u)int_base for the builtin
>>> types, in case of SC_INCLUDE_FX defined. I doubt, that this leads to
>>> performance degradation, if these operators are defined inline. Why
>>> should a compiler inline the conversion function, but not the operator?
>>>
>>> For the LRM, I think no changes are required in that case. The LRM
>>> simply lists the possible expressions for all combinations. It's up
>>> to the implementation to get this right.
>>>
>>> So I see two options here:
>>> 1) change sc_fxval(_fast) in the LRM to have 'explicit' constructors
>>> from the builtin (integral) types (cleaner, but with slight
>>> backwards comp. issues)
>>> 2) add operators to sc_(u)int_base, when SC_INCLUDE_FX is defined
>>> and keep the LRM as it is now
>>>
>>> Greetings from Oldenburg,
>>> Philipp
>>>
>>> On 26/11/10 05:59, Bishnupriya Bhattacharya wrote:
>>>> David, All,
>>>>
>>>> In SystemC, if the SC_INCLUDE_FX macro is defined then arithmetic operations with sc_int/sc_uint throw up compile errors because of ambiguity with operators defined for sc_fxval and sc_fxval_fast.
>>>>
>>>> The issue is demonstrated with the simple program below.
>>>>
>>>> #define SC_INCLUDE_FX
>>>> #include <systemc.h>
>>>>
>>>> int sc_main(int argc, char * argv[]) {
>>>> sc_int<20> x, y;
>>>>
>>>> #ifdef WORK_AROUND
>>>> y = (sc_int<20>)x + (sc_int<20>)y; #else
>>>> y = x + 1; <----------------- compile error on this line #endif
>>>>
>>>> sc_start(25);
>>>> return 0;
>>>> }
>>>>
>>>> "$TESTDIR/simple.cpp", line 11: error: more than
>>>> one operator "+" matches these operands:
>>>> built-in operator "arithmetic + arithmetic"
>>>> function "sc_dt::operator+(const sc_dt::sc_int_base &, const
>>>> sc_dt::sc_fxval &)"
>>>> function "sc_dt::operator+(const sc_dt::sc_int_base &, const
>>>> sc_dt::sc_fxval_fast &)"
>>>> operand types are: sc_dt::sc_int<20> + int
>>>> y = x + 1;
>>>>
>>>> I've discussed this with Andy Goodrich. The compile error is happening because the sc_int class does not provide explicit operators for sc_int and native C++ data types, but rely on the automatic conversion of a sc_int to long long, and then for the native operators to kick in - this is for performance reasons, such that sc_int math is faster because the native hardware gets used rather than executing an overloaded operator. If the fixed point types are involved, then it creates ambiguity because it defines operator * for sc_int_base and sc_fxval, and the native types can all be converted to sc_fxval and to sc_fxval_fast.
>>>>
>>>> The workaround is to do the explicit casting as above, which can get quite tedious for long expressions.
>>>>
>>>> Fisrt question: Is there anything that can be done? One fix would be to define all the operators for sc_int and sc_uint and the native C++ types, but that would defeat the purpose of sc_int and sc_uint in the first place, which is to do things native. Is there any other way to make this work?
>>>>
>>>> Second, if we have to live with this behavior, then I think it makes sense for the LRM to explicitly mention this issue if SC_INCLUDE_FX is defined. I looked in the LRM, but could not find any such mention - I hope I didn't miss it.
>>>>
>>>> Thoughts?

-- 
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://www.offis.de/
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.