C backend incorrectly computing 32bit/64bit integer values in the absence of casting about orcc HOT 4 OPEN

I've pushed a fix for this. What is does this is: previously when compiling:

package cal;

actor CastActor() ==> :
my_action: action ==>
  var
    uint(size=32) a,
    uint(size=32) b,
    uint(size=64) c,
    uint(size=32) d
  do
    a := 1073741824;
    b := 8;
    c := a * b;
    d := c / 8;
    println("d: " + d);
  end
end

The generated C is:

static void my_action() {
    u32 a;
    u32 b;
    u64 c;
    u32 d;
    a = 1073741824;
    b = 8;
    c = a * b;
    d = c / 8;
    printf("d: %u\n", d);
}

The generated C now is:

static void my_action() {
    u32 a;
    u32 b;
    u64 c;
    u32 d;
    a =  1073741824;
    b =  8;
    c = (u64) a * b;
    d = (u32) c / 8;
    printf("d: %u\n", d);
}

The output of the compiled C used to be:

d: 0

It now is:

d: 1073741824

Code review for #136 anyone?

from orcc.

Hello Rob,

thats great, have you tested other applications to see if the casting breaks something ?

Cheers,
Endri

On 05 Oct 2015, at 21:31, Rob Stewart [email protected] wrote:

I've pushed a fix for this. What is does this is: previously when compiling:

package cal;

actor CastActor() ==> :
my_action: action ==>
var
uint(size=32) a,
uint(size=32) b,
uint(size=64) c,
uint(size=32) d
do
a := 1073741824;
b := 8;
c := a * b;
d := c / 8;
println("d: " + d);
end
end
The generated C is:

static void my_action() {
u32 a;
u32 b;
u64 c;
u32 d;
a = 1073741824;
b = 8;
c = a * b;
d = c / 8;
printf("d: %u\n", d);
}
The generated C now is:

static void my_action() {
u32 a;
u32 b;
u64 c;
u32 d;
a = 1073741824;
b = 8;
c = (u64) a * b;
d = (u32) c / 8;
printf("d: %u\n", d);
}
The output of the compiled C used to be:

d: 0
It now is:

d: 1073741824
Code review for #136 #136 anyone?

—
Reply to this email directly or view it on GitHub #135 (comment).

from orcc.

Hi Endri,

Thanks, yes I'll test this with some of our applicaions and the applications in orc-apps. If it all works out, I'll push to orcc/master, rather than cluttering git history with a pull request.

from orcc.

It seems like the theory behind the original problem as well as the proposed fix operate on wrong assumptions about the meaning of C code. In the expression (u64) a * b, it is not the result of the multiplication that is cast, but rather the operand a. This is due to the cast operator having a higher precedence than multiplication: https://en.cppreference.com/w/c/language/operator_precedence

Then, because one of the operands is 64 bit and another 32 bit, the latter is expanded to 64 bit by the C compiler, and consequently the multiplication is done on two 64 bit numbers.

Just based on the discussion here, it appears the proposed fix would generate u64 y = (u64) e + f * g, when all of e, f and g are 32 bit. This would effectively cast only e to 64 bit while the multiplication of f and g would be performed in 32 bit mode. Even modifying the output to actually match the intention of the fix to u64 y = (u64) (e + f * g) would not help: all the operations would be performed in 32 bit mode then. If you really want to be consistent, you should cast each operand in an operation separately (some contractions are still possible by taking into account C operator precedence).

from orcc.