[bug] Incorrect optimization on constant propagation and eliminate-local-multi-store

[mitdemverkaufer]

The spirv-opt tool incorrectly optimizes the modulo operation with constant operands when enabling the --ccp and --eliminate-local-multi-store passes.

Description of the issue

The following shader is a minimal example that reproduces the issue:

#version 310 es

precision highp float;
precision highp int;

layout(location = 0) out vec4 fragColor;

void main() {
   int t = 300000000;
   uint x = uint(t % 2000000001);   // The expected value of x is 300000000
   float ret = float(x % 2u);     // The expected value of ret is 0.0, yet the actual result is 1.0
   fragColor = vec4(ret, ret, ret, 1.0);
}

The expected value of ret is 0.0, because the value of x is 300000000, which is divisible by 2. So the expected color of the fragment is (0.0, 0.0, 0.0, 1.0), i.e., a black fragment.

However, when optimizing the compiled spv file with the --ccp and --eliminate-local-multi-store passes, the fragment color is incorrectly white, i.e., (1.0, 1.0, 1.0, 1.0).

Note that the constant values and intermediate computations are within valid ranges for the respective types, so it should not be due to the undefined behavior.

Further investigation of the binary

I further use the RenderDoc tool to debug the shader. The following is the disassembly of the shader:

void main() {
  int* t;
  uint* x;
  float* ret;
  *t = 300000000;
  int _46 = SMod(300000000, 2000000001);
  uint _47 = Bitcast(-1994967295);
  *x = _47;
  uint _60 = UMod(_47, 2);
  float _61 = ConvertUToF(_60);
  *ret = _61;
  float4 _75 = CompositeConstruct({_61, _61, _61, 1.0000});
  *fragColor = _75;
  return;
}

We can see that the value of variable x is assigned from that of variable _47, which is Bitcast(-1994967295), or 2300000001 after being bit-cast to unsigned integer.
2300000001 is equal to 2000000001 + 300000000, which is equivalent to adding two operands of the modulo operation.

Such a value of x is different from the expected operation of 300000000 % 2000000001 = 300000000 as in the semantics of the glsl shader.

Later, the value of x is used in the modulo operation with 2, which gives 2300000001 % 2 = 1, causing the value of ret to be 1.0 instead of the expected 0.0.

Steps to reproduce

Minimal commands to compile & optimize the glsl shader to spv:

glslang -gVS -o frag.non-opt.spv --target-env vulkan1.3 [file-of-glsl-shader-above].frag
spirv-opt frag.non-opt.spv -o frag.spv --eliminate-local-multi-store --ccp

I provide a miminal package that reproduces the issue. The package contains an application that draws a triangle with the incorrectly optimized spirv shader. It also describes the environment, hardware, and steps to reproduce the issue. The capture file of RenderDoc is also included. You can download the package from here

[s-perron]

Look like I fixed the surface issue with #5561. However, I do not understand how we were getting the earlier result in the first place. I did not think that the OpSMod would have been folded, so it seems like there might be another bug that is now hidden. I'll leave this open while I try to determine what was happening before my change.

[s-perron]

The code that originally folded the OpSMod, would do an OSRem and then try to "fix" the value to account for the difference in definition:

SPIRV-Tools/source/opt/fold.cpp

Lines 109 to 111 in 784b064

	int32_t rem = BinaryOperate(spv::Op::OpSRem, a, b);
	int32_t b_prim = static_cast<int32_t>(b);
	return (rem + b_prim) % b_prim;

The problem is that the third line overflows in this case, and give an incorrect value. Since we have a new implementation for OpSMod folding, which does not have this problem. I will simply delete that code.