KhronosGroup / glslang

Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator.
Other
2.98k stars 822 forks source link

Non-semantic OpExtInst is incorrectly placed before OpPhi in a block #3665

Closed JuanDiegoMontoya closed 1 month ago

JuanDiegoMontoya commented 1 month ago

I initially raised this issue for the validation layers, but it turns out this is actually a codegen issue. The original issue can be found here. This was further clarified by the specification for SPV_KHR_non_semantic_info, which describes precisely how this is invalid.

Below is mostly copy-pasted from the linked issue.

Environment

The Issue I have a shader that has been compiled with glslang's C++ API with debug info. Debug info has been enabled through the following code:

shader.setDebugInfo(true);
...
auto options = glslang::SpvOptions{
    .generateDebugInfo = true,
    .stripDebugInfo = false,
    .disableOptimizer = true,
    .emitNonSemanticShaderDebugInfo = true,
    .emitNonSemanticShaderDebugSource = true,
};

The full code for compiling shaders can be found here.

Attempting to create a shader module with the produced SPIR-V (attached at the end) results in this message from the validation layers:

Validation Error: [ VUID-VkShaderModuleCreateInfo-pCode-08737 ] | MessageID = 0xa5625282 | vkCreateShaderModule(): pCreateInfo->pCode (spirv-val produced an error): OpPhi must appear within a non-entry block before all non-OpPhi instructions (except for OpLine, which can be mixed with OpPhi). %2234 = OpPhi %bool %2187 %2160 %2233 %2189 . The Vulkan spec states: If pCode is a pointer to SPIR-V code, pCode must adhere to the validation rules described by the Validation Rules within a Module section of the SPIR-V Environment appendix (https://vulkan.lunarg.com/doc/view/1.3.283.0/windows/1.3-extensions/vkspec.html#VUID-VkShaderModuleCreateInfo-pCode-08737)

The instruction that it's complaining about is located in this block:

[2002]  %2190 = OpLabel
[2003]  %2235 = OpExtInst %19 %1 DebugScope %388
[2004]  %2234 = OpPhi %146 (%2187 : %2160) (%2233 : %2189)
[2005]  OpSelectionMerge %2237 None
[2006]  OpBranchConditional %2234 %2236 %2237

Expected Behavior OpExtInst should not be placed before OpPhi in a block.

Additional Info The shader in question can be found here: https://github.com/JuanDiegoMontoya/Frogfood/blob/main/data/shaders/visbuffer/CullMeshlets.comp.glsl The affected SPIR-V is attached in bug.zip.

memory-thrasher commented 1 month ago

Seconded. I've compiled a much more limited test case to help.

opPhi.vert.glsl

#version 450

void main() {
  const bool i = gl_VertexIndex > 1 && gl_VertexIndex < 5;
  gl_Position = vec4(0, 0, 0, 0);
}

opPhi.cpp

#include <vulkan/vulkan.hpp>
#include <iostream>

#include "opPhi.vert.hpp"

int main(int argc, char** argv) {
  vk::Result res;
  vk::ApplicationInfo appI { "Op Phi test cast", 0, "null", 0, (1 << 22) | (3 << 12) | 283 };
  vk::InstanceCreateInfo ici { (vk::InstanceCreateFlags)0, &appI };
  ici.enabledLayerCount = 1;
  const char* validation = "VK_LAYER_KHRONOS_validation";
  ici.ppEnabledLayerNames = &validation;
  vk::Instance vkInstance;
  res = vk::createInstance(&ici, NULL, &vkInstance);
  std::cout << res << std::endl;
  vk::PhysicalDevice pv;
  uint32_t cnt = 1;
  res = vkInstance.enumeratePhysicalDevices(&cnt, &pv);
  std::cout << res << std::endl; //expect result 5: we only asked for 1 pv
  float p = 1;
  //we don't actually need a queue to compile the shader so just pick idx 0
  vk::DeviceQueueCreateInfo dqci { {}, 0, 1, &p };
  vk::DeviceCreateInfo dci { (vk::DeviceCreateFlags)0, 1, &dqci };
  vk::Device dev;
  res = pv.createDevice(&dci, NULL, &dev);
  std::cout << res << std::endl;
  vk::ShaderModuleCreateInfo smci { {}, sizeof(opPhi_vert), opPhi_vert };
  vk::ShaderModule module;
  res = dev.createShaderModule(&smci, NULL, &module);
  std::cout << res << std::endl;
}

build.sh

glslangValidator -V --target-env vulkan1.3 -gVS opPhi.vert.glsl -o opPhi.vert.hpp --vn opPhi_vert || exit 1

clang++ "-I$VULKAN_SDK/include" --std=c++20 -Werror -Wall opPhi.cpp -o opPhi -lvulkan

output:

vk::Result:0
vk::Result:5
vk::Result:0
VUID-VkShaderModuleCreateInfo-pCode-08737(ERROR / SPEC): msgNum: -1520283006 - Validation Error: [ VUID-VkShaderModuleCreateInfo-pCode-08737 ] | MessageID = 0xa5625282 | vkCreateShaderModule(): pCreateInfo->pCode (spirv-val produced an error):
OpPhi must appear within a non-entry block before all non-OpPhi instructions (except for OpLine, which can be mixed with OpPhi).
  %58 = OpPhi %bool %50 %15 %57 %51
. The Vulkan spec states: If pCode is a pointer to SPIR-V code, pCode must adhere to the validation rules described by the Validation Rules within a Module section of the SPIR-V Environment appendix (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-VkShaderModuleCreateInfo-pCode-08737)
    Objects: 0
vk::Result:0
memory-thrasher commented 1 month ago

my glslangValidator, from the current main branch, although the most recent tag gets the same result.

$ glslangValidator --version
Glslang Version: 11:14.3.0
ESSL Version: OpenGL ES GLSL 3.20 glslang Khronos. 14.3.0
GLSL Version: 4.60 glslang Khronos. 14.3.0
SPIR-V Version 0x00010600, Revision 1
GLSL.std.450 Version 100, Revision 1
Khronos Tool ID 8
SPIR-V Generator Version 11
GL_KHR_vulkan_glsl version 100
ARB_GL_gl_spirv version 100
memory-thrasher commented 1 month ago

as a temporary measure, I am simply disabling the generation of debug info by removing -gVS, which removes the offending OpExtInst. There should be an equivalent option in the runtime compilation paradigm.

arcady-lunarg commented 1 month ago

Thanks for providing the reproduction case, I am pretty sure this is a regression as well. I'll take a look.

arcady-lunarg commented 1 month ago

Fortunately this should be pretty easy to fix. We even had a test that tested it.

memory-thrasher commented 1 month ago

Confirmed working on my end, built locally from main. Thanks.

JuanDiegoMontoya commented 1 month ago

Works here too, thanks!