[spirv-opt] ssa-rewrite producing invalid spir-v for shader

[danginsburg]

Given the attached shader ssa_rewrite_bug.spv using spirv-opt SPIRV-Tools v2024.4 v2024.4.rc2-0-g4d2f0b40 (Vulkan SDK 1.4.304.0) and running:

spirv-opt --ssa-rewrite ssa_rewrite_bug.spv -o opt.spv

spirv-val on the output shader gives the following error:

error: line 2617: Result type cannot be OpTypeImage
  %15221 = OpPhi %79 %6612 %6611 %6614 %6613

I hit this as part of running our shaders through -Os and narrowed it down to the ssa-rewrite pass.

ssa_rewrite_bug.zip

[alan-baker]

The validator isn't catching it, but the input SPIR-V is invalid.

VUID-StandaloneSpirv-OpTypeImage-06924

Objects of types OpTypeImage, OpTypeSampler, OpTypeSampledImage, OpTypeAccelerationStructureKHR, and arrays of these types must not be stored to or modified

In this case there are function scope variables of OpTypeImage. I expect the input shader needs more legalization. Unfortunately in SPIR-V, the code pattern used is invalid, e.g. (pseudo code):

image i; // %6153 in the SPIR-V
if (cond) {
  i = image1;
} else {
  i = image2;
}
vec4 res = sampleGrad(i, sampler, ...); // %6632 in the SPIR-V

Because SPIR-V cannot select between images (nor are variable pointers usable in UniformConstant storage class) I think the only valid code would be to sink the sampling into the if. Works here since it's a grad operation. In other cases it would need to be converted to a grad to avoid non-uniform derivatives. I'm not sure any legalization does this currently.

[danginsburg]

Ah, thank you. The pattern you identified is similar to what the shader is doing, yes. This spir-v was generated by slang and I presumed it was correct because it was passing spirv-val. Do I understand correctly that we have the following bugs:

• spirv-val is missing check for VUID-StandaloneSpirv-OpTypeImage-06924
• Legalization needs a pass to handle hoisting the sample into the control flow
• slang needs to either use that legalization pass (or its own), but can't generate this code

[alan-baker]

Yes, I think that's right.

[alan-baker]

Actually that VUID is checked under the Vulkan environment. So you validate with --target-env vulkan1.3 in this case it catches it:

error: line 2623: [VUID-StandaloneSpirv-OpTypeImage-06924] Cannot store to OpTypeImage, OpTypeSampler, OpTypeSampledImage, or OpTypeAccelerationStructureKHR objects
  OpStore %6153 %6612

[s-perron]

We'll leave this open in case someone wants to write the pass to replicate the code. Some of the building block might already exist in the ReplaceDescArrayAccessUsingVarIndex pass. My team will not get to this, but if someone from the slang teams want to try, we can review it.