Jump to content
jserot

Bug on ?: operator for sc_int class ?

Recommended Posts

Hi,

I just stumbled on this.

The first form (with an "if") compiles ok.

The second one (with the "?:") fails with this message :

g++ -Wall -I. -I/Library/SystemC/systemc-2.3.0/include -Wno-deprecated -c -o foo.o foo.cpp

foo.cpp: In function 'sc_dt::sc_int<n> f_abs(sc_dt::sc_int<n>) [with int n = 8]':

foo.cpp:14: instantiated from here

foo.cpp:9: error: operands to ?: have different types

make: *** [foo.o] Error 1

Could this be a bug ?

Any help will be appreciated.

Thanks in advance.

Jocelyn

------ START of CODE SAMPLE --------

#define SC_INCLUDE_FX

#include <systemc.h>

template<int n>

sc_int<n> f_abs(sc_int<n> x) {

if ( x <0 ) return 0-x; else return x; // OK :-)

return x<0 ? 0-x : x; // NOK :-(

};

int sc_main (int argc , char *argv[]) {

sc_int<8> x=2, y=-3;

cout << "abs(" << x << ")=" << f_abs(x) << endl;

cout << "abs(" << y << ")=" << f_abs(y) << endl;

return 0;

}

------ END of CODE SAMPLE ----------

Share this post


Link to post
Share on other sites

Hello,

Apologies for my confusion, but two points are not clear.

A:

template<int n>

sc_int<n> f_abs(sc_int<n> x) {

if ( x <0 ) return 0-x; else return x; // OK :-)

return x<0 ? 0-x : x; // NOK :-(

};

Effectively there are two return statements in the same method.

Is this legal ?

2.

Both C/C++ have a feature, the name of which I forget right

now (something like undefined ...) that, if present in the code,

confuses the compiler. Please check this.

Share this post


Link to post
Share on other sites

Thanks for your reply.

Sorry. My code was confusing. Of course, when compiling, i comment out one of the two lines containing the return statement.

To be clearer :

This compiles ok :

template<int n>

sc_int<n> f_abs(sc_int<n> x) {

if ( x <0 ) return 0-x; else return x;

};

But this fails to compile :

template<int n>

sc_int<n> f_abs(sc_int<n> x) {

return x<0 ? 0-x : x;

};

Share this post


Link to post
Share on other sites

It's not a bug in SystemC, C++, nor in your compiler. It's just a restriction of the conditional operator ?: in C++. As the compiler error tells you, you need to have expressions of the same type in both operands. The list of allowed conversions is quite restricted (see e.g. here).

Now, sc_int<N> is of class type, whereas the arithmetics are done interallly based on 64-bit integral types. Therefore, you need to cast the result back to sc_int<N> explicitly, or trick both operands to be artihmetic expressions:

 x<0 ? sc_int<n>(-x) : x;

or

 x<0 ? -x : +x;

Greetings from Oldenburg,

Philipp

NB: I replaced the explicit subtraction "0-x" with a unary minus for clarity and efficiency.

Share this post


Link to post
Share on other sites

Thanks a lot Philipp !

I was suspecting sth like this but was not aware doing arithmetic on one operand was actually changing its type (compared to the other one).

In fact, i had searched the sc_int class for an (overloaded) version of the arithm operators (+, _, ...) and was a bit puzzled not to find them.

Anyway, this explains the problem.

... but incidently, does not simplify my life - since the code in question is actually not written by hand but generated by a compiler (more details on this here, in case you would be interested : http://dream.univ-bpclermont.fr/index.php/en/softmenu/caph.html). So this means that i will have to detect and correct these situations on a ad-hoc, type-based, basis :-S

Thanks again for your help

Best wishes

Jocelyn

Share this post


Link to post
Share on other sites

Hi Jocelyn,

assuming you are the co-author of the compiler (Jocelyn Serot) then the fix should surely be to fix the compiler - by generating an if statement, instead of using the conditional operator?

regards

Alan

P.S. I was going to suggest you report a bug to the authors of CAPH - but if you're one of the authors you can report it to yourself :-)

Share this post


Link to post
Share on other sites

You guessed right, Alan ;)

The bug will be fixed in vers 1.6 (not by replacing the cond op by an "if" but by using the result of the type inference phase to insert the proper type cast when requested).

Best regards

Jocelyn

Share this post


Link to post
Share on other sites

Yes, synthesis of C++ data types (like the SystemC ones) is hard and comes with a lot of corner cases.

Although I don't know CAPH, two comments:

  • You mention a "type inference phase". The rules for (integer) arithmetics are quite complex in C (and thus in C++). Make sure, that you get the integer promotion rules and the (un)signedness correct. This is quite hard to debug. Hopefully, a good C++ frontend handles most of the work for you here.
  • Inserting the "correct" cast will be very difficult as well. You should rely on the unary operator '+' in this particular case instead: When you detect an expression in one of the operands, make the other one an expression as well by prepending a '+'. This should work for all reasonable arithmetic types and allows you to rely on the C++ rules. Moreover, it will be more efficient (during post-synthesis simulation).

Greetings from Oldenburg,

Philipp

Share this post


Link to post
Share on other sites

Philip,

The type inference phase is carried out on a backend-independant representation.

In this particular case, the tricky thing to do is to map the types computed at this level to SystemC types because, as you rightly puts it, there a significant number of corner cases (and the VHDL backend is even worst..).

The solution you suggest (inserting unary + or equiv) will unfortunately not work in all cases, because _any kind_ of expression can appear as an argument of the ternary cond operator (sorry, it's difficult to be more explicit w/o showing the intermediate representation and associated transfo rules which are written ... in OCaml ;))

Thanks anyway for your clever comments and suggestions !

Jocelyn

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×