Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using gl_ObjectToWorld3x4EXT and gl_ObjectToWorldEXT at the same time leads to invalid SPIR-V #3007

Open
nvmheyer opened this issue Aug 18, 2022 · 5 comments
Labels
bug GLSL/ESSL sev:miscompile Given a valid input, glslang produces incorrect or invalid SPIR-V SPIR-V

Comments

@nvmheyer
Copy link

#version 460
#extension GL_EXT_ray_tracing : require


void main()
{
  const vec3 nrm      =  vec3(1.0,0,0);
  const vec3 worldNrm = normalize(mat3(gl_ObjectToWorld3x4EXT)* nrm); 

  const vec3 pos      = vec3(1.0, 1.0, 1.0);
  const vec3 worldPos = vec3(gl_ObjectToWorldEXT * vec4(pos, 1.0)); 

}
"
               OpSourceExtension "GL_EXT_ray_tracing"
               OpName %main "main"
               OpName %worldNrm "worldNrm"
               OpName %gl_ObjectToWorld3x4EXT "gl_ObjectToWorld3x4EXT"
               OpName %worldPos "worldPos"
               OpModuleProcessed "client vulkan100"
               OpModuleProcessed "target-env spirv1.5"
               OpModuleProcessed "target-env vulkan1.2"
               OpModuleProcessed "entry-point main"
               OpDecorate %gl_ObjectToWorld3x4EXT BuiltIn ObjectToWorldNV
       %void = OpTypeVoid
          %4 = OpTypeFunction %void
      %float = OpTypeFloat 32
    %v3float = OpTypeVector %float 3
%_ptr_Function_v3float = OpTypePointer Function %v3float
%mat4v3float = OpTypeMatrix %v3float 4
    %v4float = OpTypeVector %float 4
%mat3v4float = OpTypeMatrix %v4float 3
%_ptr_Input_mat4v3float = OpTypePointer Input %mat4v3float
%gl_ObjectToWorld3x4EXT = OpVariable %_ptr_Input_mat4v3float Input
%mat3v3float = OpTypeMatrix %v3float 3
    %float_1 = OpConstant %float 1
    %float_0 = OpConstant %float 0
         %28 = OpConstantComposite %v3float %float_1 %float_0 %float_0
         %34 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
               OpLine %1 24 11
       %main = OpFunction %void None %4
          %6 = OpLabel
   %worldNrm = OpVariable %_ptr_Function_v3float Function
   %worldPos = OpVariable %_ptr_Function_v3float Function
               OpLine %1 28 0
         %16 = OpLoad %mat4v3float %gl_ObjectToWorld3x4EXT
         %17 = OpTranspose %mat3v4float %16
         %19 = OpCompositeExtract %v4float %17 0
         %20 = OpVectorShuffle %v3float %19 %19 0 1 2
         %21 = OpCompositeExtract %v4float %17 1
         %22 = OpVectorShuffle %v3float %21 %21 0 1 2
         %23 = OpCompositeExtract %v4float %17 2
         %24 = OpVectorShuffle %v3float %23 %23 0 1 2
         %25 = OpCompositeConstruct %mat3v3float %20 %22 %24
         %29 = OpMatrixTimesVector %v3float %25 %28
         %30 = OpExtInst %v3float %2 Normalize %29
               OpStore %worldNrm %30
               OpLine %1 32 0
         %32 = OpLoad %mat4v3float %gl_ObjectToWorld3x4EXT
         %33 = OpTranspose %mat3v4float %32
         %35 = OpMatrixTimesVector %v3float %33 %34
         %36 = OpCompositeExtract %float %35 0
         %37 = OpCompositeExtract %float %35 1
         %38 = OpCompositeExtract %float %35 2
         %39 = OpCompositeConstruct %v3float %36 %37 %38
               OpStore %worldPos %39
               OpReturn
               OpFunctionEnd

leads to invalid SPIR-V that the validator will later complain about:

ERROR: UNASSIGNED-CoreValidation-Shader-InconsistentSpirv
 --> Validation Error: [ UNASSIGNED-CoreValidation-Shader-InconsistentSpirv ] Object 0: handle = 0x1ebb13ff470, type = VK_OBJECT_TYPE_DEVICE; | MessageID = 0x6bbb14 | SPIR-V module not valid: Expected column type of the matrix to be equal to Result Type: MatrixTimesVector
  %35 = OpMatrixTimesVector %v3float %33 %34

What happens is that gl_ObjectToWorldEXT will be applied a transpose (in the SPIR-V) when it should not.

This seems to be very related to an older issue: #2921

This issue is present in the latest Vulkan SDK, 1.3.216.0

gl_WorldToObjectEXT and gl_WorldToObject3x4EXT are likely also affected.

@nvmheyer
Copy link
Author

Interesting data point; when swapping the usage of gl_ObjectToWorldEXT and gl_ObjectToWorld3x4EXT, it does seem to do the right thing:

#version 460
#extension GL_EXT_ray_tracing : require


void main()
{
  // Computing the coordinates of the hit position
  const vec3 pos      = vec3(1.0, 1.0, 1.0);
  const vec3 worldPos = vec3(gl_ObjectToWorldEXT * vec4(pos, 1.0));  // Transforming the position to world space

  // Computing the normal at hit position
  const vec3 nrm      =  vec3(1.0,0,0);
  const vec3 worldNrm = normalize(mat3(gl_ObjectToWorld3x4EXT)* nrm);  // Transforming the normal to world space

}
"
               OpSourceExtension "GL_EXT_ray_tracing"
               OpName %main "main"
               OpName %worldPos "worldPos"
               OpName %gl_ObjectToWorldEXT "gl_ObjectToWorldEXT"
               OpName %worldNrm "worldNrm"
               OpModuleProcessed "client vulkan100"
               OpModuleProcessed "target-env spirv1.5"
               OpModuleProcessed "target-env vulkan1.2"
               OpModuleProcessed "entry-point main"
               OpDecorate %gl_ObjectToWorldEXT BuiltIn ObjectToWorldNV
       %void = OpTypeVoid
          %4 = OpTypeFunction %void
      %float = OpTypeFloat 32
    %v3float = OpTypeVector %float 3
%_ptr_Function_v3float = OpTypePointer Function %v3float
%mat4v3float = OpTypeMatrix %v3float 4
%_ptr_Input_mat4v3float = OpTypePointer Input %mat4v3float
%gl_ObjectToWorldEXT = OpVariable %_ptr_Input_mat4v3float Input
    %v4float = OpTypeVector %float 4
    %float_1 = OpConstant %float 1
         %17 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
%mat3v4float = OpTypeMatrix %v4float 3
%mat3v3float = OpTypeMatrix %v3float 3
    %float_0 = OpConstant %float 0
         %36 = OpConstantComposite %v3float %float_1 %float_0 %float_0
               OpLine %1 24 11
       %main = OpFunction %void None %4
          %6 = OpLabel
   %worldPos = OpVariable %_ptr_Function_v3float Function
   %worldNrm = OpVariable %_ptr_Function_v3float Function
               OpLine %1 28 0
         %14 = OpLoad %mat4v3float %gl_ObjectToWorldEXT
         %18 = OpMatrixTimesVector %v3float %14 %17
         %19 = OpCompositeExtract %float %18 0
         %20 = OpCompositeExtract %float %18 1
         %21 = OpCompositeExtract %float %18 2
         %22 = OpCompositeConstruct %v3float %19 %20 %21
               OpStore %worldPos %22
               OpLine %1 32 0
         %25 = OpLoad %mat4v3float %gl_ObjectToWorldEXT
         %26 = OpTranspose %mat3v4float %25
         %28 = OpCompositeExtract %v4float %26 0
         %29 = OpVectorShuffle %v3float %28 %28 0 1 2
         %30 = OpCompositeExtract %v4float %26 1
         %31 = OpVectorShuffle %v3float %30 %30 0 1 2
         %32 = OpCompositeExtract %v4float %26 2
         %33 = OpVectorShuffle %v3float %32 %32 0 1 2
         %34 = OpCompositeConstruct %mat3v3float %29 %31 %33
         %37 = OpMatrixTimesVector %v3float %34 %36
         %38 = OpExtInst %v3float %2 Normalize %37
               OpStore %worldNrm %38
               OpReturn
               OpFunctionEnd

@dgkoch
Copy link
Contributor

dgkoch commented Aug 18, 2022

cc @andfau-arm who provided the fix for #2921

@andfau-arm
Copy link
Contributor

andfau-arm commented Aug 19, 2022

Looking at my patch from #2955 (which fixed #2921) again, I wonder if it could be somehow related to the "forced type" part here in GlslangToSpv.cpp:

    if (mayNeedToReuseBuiltIn) {
        auto iter = builtInVariableIds.find(uint32_t(builtIn));
        if (builtInVariableIds.end() != iter) {
            id = iter->second;
            symbolValues[symbol->getId()] = id;
            if (forcedType.second != spv::NoType)
                forceType[id] = forcedType.second;
            return id;
        }
    }

Do we "force" the type (the transposed matrix type) for both uses of the built-in if they come in the "wrong" order?

@andfau-arm
Copy link
Contributor

If that is indeed the problem, then the fix would be to use something other than the SPIR-V ID to track values that need "forced" types. The 3x4 and 4x3 matrices need to have the same SPIR-V ID (because we can only have one OpVariable), but they must have different types, one "forced".

@timo-oster
Copy link

Just chiming in here to say that I'm seeing the same problem with gl_WorldToObjectEXT and gl_WorldToObject3x4EXT.

In my particular case, I was able to work around the problem by replacing this line:

vec3 objectPos = gl_WorldToObjectEXT * vec4(result.worldPosition, 1);

with this line:

vec3 objectPos = vec4(result.worldPosition, 1) * gl_WorldToObject3x4EXT;

This is the first use of gl_WorldToObject* in a closest hit shader. Later gl_WorldToObject3x4EXT is used again during the same shader invocation.

@arcady-lunarg arcady-lunarg added the sev:miscompile Given a valid input, glslang produces incorrect or invalid SPIR-V label Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug GLSL/ESSL sev:miscompile Given a valid input, glslang produces incorrect or invalid SPIR-V SPIR-V
Projects
None yet
Development

No branches or pull requests

6 participants