Handle signed arithmetic edge cases that would be UB in Metal

[teoxoy]

The following operations would overflow with signed integer operands. The results are what the WGSL spec requires.

div: T::MIN / -1 = T::MIN
rem: T::MIN % -1 = 0
abs: abs(T::MIN) = T::MIN
neg: -T::MIN = T::MIN

Related: #6666 / #6959

[jimblandy]

Have we looked into which of these cases need attention in the other backend languages?

[teoxoy]

I haven't but suspect HLSL/GLSL would need them.

[jamienicol]

Metal

Spec notes

• div: "Division on integer types that result in a value that lies outside of the range bounded by the maximum and minimum representable values of the integer type, such as TYPE_MIN/-1 for signed integer types or division by zero, does not cause an exception but results in an unspecified value"
  • So not undefined behaviour, but we must handle this to comply with the WGSL spec
• rem: If one or both operands are negative, the results are undefined.
• abs: Just says "Returns |x|" - Perhaps this is covered by general "The result of signed integer overflow is undefined" statement elsewhere in the spec, but that is from a different section.
• neg: Again nothing specific but presumably this is indeed covered by "the result of signed integer overflow is undefined" as it's from the same section (on operators)

What does Tint do?

For this wgsl:

fn do_stuff(x: i32) {
    var div_res = x / -1;
    var mod_res = x % -1;
    var abs_res = abs(x);
    var neg_res = -x;
}

Tint outputs:

int tint_unary_minus(const int v) {
  return select(-v, v, v == -2147483648);
}

int tint_div(int lhs, int rhs) {
  return (lhs / select(rhs, 1, bool((rhs == 0) | bool((lhs == (-2147483647 - 1)) & (rhs == -1)))));
}

int tint_mod(int lhs, int rhs) {
  int const rhs_or_one = select(rhs, 1, bool((rhs == 0) | bool((lhs == (-2147483647 - 1)) & (rhs == -1))));
  if (any(((uint((lhs | rhs_or_one)) & 2147483648u) != 0u))) {
    return as_type<int>((as_type<uint>(lhs) - as_type<uint>(as_type<int>((as_type<uint>((lhs / rhs_or_one)) * as_type<uint>(rhs_or_one))))));
  } else {
    return (lhs % rhs_or_one);
  }
}

void do_stuff(int x) {
  int div_res = tint_div(x, -1);
  int mod_res = tint_mod(x, -1);
  int abs_res = abs(x);
  int neg_res = tint_unary_minus(x);
}

Interestingly they handle the unary minus but not abs().

Also interesting to note the -{i32::MAX} - 1 construct rather than {i32::MIN}. Presumably this means metal parses -1 as <unary neg> <unsigned literal> instead of <signed literal>. Which is something to look out for.

HLSL

Spec notes

🙃

What does Tint do?

int tint_div(int lhs, int rhs) {
  return (lhs / (((rhs == 0) | ((lhs == -2147483648) & (rhs == -1))) ? 1 : rhs));
}

int tint_mod(int lhs, int rhs) {
  int rhs_or_one = (((rhs == 0) | ((lhs == -2147483648) & (rhs == -1))) ? 1 : rhs);
  if (any(((uint((lhs | rhs_or_one)) & 2147483648u) != 0u))) {
    return (lhs - ((lhs / rhs_or_one) * rhs_or_one));
  } else {
    return (lhs % rhs_or_one);
  }
}

void do_stuff(int x) {
  int div_res = tint_div(x, -1);
  int mod_res = tint_mod(x, -1);
  int abs_res = abs(x);
  int neg_res = -(x);
}

It's interesting they don't wrap abs or neg. But then nor do they handle signed addition, subtraction, or multiplication for HLSL, whereas they do for MSL, which seems consistent.

SPIRV

Spec notes

• OpSDiv: Behavior is undefined if Operand 2 is -1 and Operand 1 is the minimum representable value for the operands' type, causing signed overflow.
• OpSRem: Behavior is undefined if Operand 2 is -1 and Operand 1 is the minimum representable value for the operands' type, causing signed overflow.
• GLOp::SAbs: Doesn't mention anything regarding overflow except "This instruction can be decorated with NoSignedWrap.", which presumably means the absence of that decoration we safely overflow to INT_MIN, which is what we want
• SNegate: same as above

What does tint do?

Won't post the whole thing but we can see they have wrapper functions for div and rem, and just use OpSNegate and SAbs directly, which agrees with my read of the spec.

    %div_res = OpVariable %_ptr_Function_int Function
    %mod_res = OpVariable %_ptr_Function_int Function
    %abs_res = OpVariable %_ptr_Function_int Function
    %neg_res = OpVariable %_ptr_Function_int Function
          %7 = OpFunctionCall %int %tint_div_i32 %x %int_n1
               OpStore %div_res %7
         %12 = OpFunctionCall %int %tint_mod_i32 %x %int_n1
               OpStore %mod_res %12
         %15 = OpExtInst %int %16 SAbs %x
               OpStore %abs_res %15
         %18 = OpSNegate %int %x
               OpStore %neg_res %18

GLSL

I might leave GLSL for a follow-up, but for posterity here's what tint does:

int tint_mod_i32(int lhs, int rhs) {
  uint v = uint((lhs == (-2147483647 - 1)));
  bool v_1 = bool((v & uint((rhs == -1))));
  uint v_2 = uint((rhs == 0));
  int v_3 = mix(rhs, 1, bool((v_2 | uint(v_1))));
  return (lhs - ((lhs / v_3) * v_3));
}
int tint_div_i32(int lhs, int rhs) {
  uint v_4 = uint((lhs == (-2147483647 - 1)));
  bool v_5 = bool((v_4 & uint((rhs == -1))));
  uint v_6 = uint((rhs == 0));
  return (lhs / mix(rhs, 1, bool((v_6 | uint(v_5)))));
}
void do_stuff(int x) {
  int div_res = tint_div_i32(x, -1);
  int mod_res = tint_mod_i32(x, -1);
  int abs_res = abs(x);
  int neg_res = -(x);
}

They don't worry about signed addition, subtraction, or multiplication either.

[cwfitzgerald]

HLSL

Spec notes

🙃

I asked on the directx discord about signed integer overflow and we'll see what they say

[cwfitzgerald]

Sad news

Signed integer overflow in DXIL is undefined, in DXBC it probably was well defined.

So we do need to handle it, it seems.