Undefined behavior for shifts of negative integers in nir_opcodes.py
As was discovered in MR !3445 (merged), there's a bunch of places in src/compiler/nir/nir_opcodes.py
where we rely on either undefined behavior or implementation-defined behavior for shifts of negative values. Section 6.5.7 ("Bitwise shift operators") of the C99 n1124 spec says:
- The result of
E1 << E2
isE1
left-shiftedE2
bit positions; vacated bits are filled with zeros. IfE1
has an unsigned type, the value of the result isE1
× 2E2
, reduced modulo one more than the maximum value representable in the result type. IfE1
has a signed type and nonnegative value, andE1
× 2E2
is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined.- The result of
E1 >> E2
isE1
right-shiftedE2
bit positions. IfE1
has an unsigned type or ifE1
has a signed type and a nonnegative value, the value of the result is the integral part of the quotient ofE1
/ 2E2
. IfE1
has a signed type and a negative value, the resulting value is implementation-defined.
Looking at the generated nir_constant_expressions.c
file, I believe at least the following opcodes suffer from these problems:
ibfe
ibitfield_extract
imad24_ir3
imadsh_mix16
imul24
-
umax_4x8
(just change the type totuint32
?) -
umin_4x8
(just change the type totuint32
?) -
umul_unorm_4x8
(just change the type totuint32
?) -
ussub_4x8
(just change the type totuint32
?) -
usadd_4x8
(just change the type totuint32
?) extract_i16
extract_i8
-
ifind_msb
(usesrc0 & (1 << bit)
instead) ihadd
imul_high
irhadd
Since the CI can catch the undefined behavior cases, the <<
users should be fixed first. It should be trivial to make a unit test for each that will trigger an error from the gitlab pre-merge CI.