Crystal: i32 + u64 produces an overflow error

[RX14]

Reproduce with -1 + 0u64

I'm not entirely sure what's going on here, but the codegen listing is as such:

  %0 = call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 -1, i64 0), !dbg !11649
  %1 = extractvalue { i64, i1 } %0, 0, !dbg !11649
  %2 = extractvalue { i64, i1 } %0, 1, !dbg !11649
  %3 = icmp ult i64 %1, -2147483648, !dbg !11649
  %4 = and i1 false, %3, !dbg !11649
  %5 = icmp ugt i64 %1, 2147483647, !dbg !11649
  %6 = or i1 false, %5, !dbg !11649
  %7 = or i1 %4, %6, !dbg !11649
  %8 = or i1 %2, %7, !dbg !11649
  %9 = call i1 @llvm.expect.i1(i1 %8, i1 false), !dbg !11649
  br i1 %9, label %overflow, label %normal, !dbg !11649

Seems like it should be using a signed add? Perhaps something to do with codegen_binary_extend_int?

[bcardiff]

The issue is that the operation is performed in the highest rank number, in this case: UInt64.
The -1i32 interpreted as i64 in an unsigned addition leads to UInt64::MAX which is greater than the max value of the target type (Int32) of the original operation.

This happens when mixing signed and unsigned numbers.

Other cases that are wrongly handled are

# should not overflow but it does
-1i8 + 0u16
-1i16 + 0u32
-1i32 + 0u64
-1i64 + 0.to_u128

# should overflow but it does not
0u16 + (-1i8)
0u32 + (-1i16)
0u64 + (-1i32)
0.to_u128 + (-1i32)
0u8 + (-1i8)
0u16 + (-1i16)
0u32 + (-1i32)
0u64 + (-1i64)
0.to_u128 + (-1.to_i128)

A solution for example could be that when doing a + b. If a is unsigned and b is signed do:

• if b < 0 and a < -b then overflow (the result is expected to be unsigned)
• otherwise perform the operation as today.

this will solve the "should overflow but it does not".

Something similar probably could be done in the other case to distinguis UInt::Max from -1 as a result, and of course other values like UInt::Max-1 from -2, etc.

[asterite]

The issue is that the operation is performed in the highest rank number

If we do the operation always on the right-hand side type, downcasting if needed the left-hand side operand, would it work fine?

[RX14]

I wonder how LLVM will optimize these checks... binary size and LLVM size issues worry me if we're doing four checks for adds between different types.

Perhaps we should change the semantic to be a + b == a + typeof(a).new(b) and always overflow if b doesn't ft in a. Then there's only one check per add.

This would change the semantic of 10u32 + -5i32 but does this really matter? If doing it mathematically correctly can't produce small code then it might be a worthwhile tradeoff.

[waj]

In C# that operation doesn't even compile: Operator '+' cannot be applied to operands of type 'int' and 'ulong'
I couldn't find a reference yet, but it seems the operation always returns a type that could fit the result. For example:

• Int16 + UInt32 => Int64
• Int32 + UInt32 => Int64
• Int32 + UInt64 => compile type error

[RX14]

I wouldn't want to change how arithmetic operators are typed, just their semantics.

[bcardiff]

For references: In C++ there is a _short_ implementation to deal with all these cases https://github.com/dcleblanc/SafeInt/blob/master/SafeInt.hpp 🎉