shader-slang / slang

Making it easier to work with shaders
MIT License
1.78k stars 159 forks source link

SPIRV - pAccessChain result type (OpTypePointer) does not match the type that results from indexing into the base <id> (OpTypePointer) #4380

Open BeastLe9enD opened 2 weeks ago

BeastLe9enD commented 2 weeks ago

Hey, when I store a pointer inside a structure, I get errors from spirv-val.

struct Constants {
  uint *p;
};

[[vk::push_constant]] Constants constants;

[[vk::binding(0)]] RWTexture2D<uint> my_texture;

struct MyType {
  uint *_p;

  __init(uint* p) {
    _p = p;
  }

  void write_to(RWTexture2D<uint> t) {
    t[uint2(0, 0)] = *_p;
  }
}

[numthreads(1, 1, 1)]
void main() {
  MyType(constants.p).write_to(my_texture);
}

compiling it with slangc.exe -emit-spirv-directly .\bruh.slang -o bruh.spv | spirv-val.exe .\bruh.spv returns errors from spirv-val:

error: line 49: OpVariable 60: expected AliasedPointer or RestrictPointer for PhysicalStorageBuffer pointer.
  %60 = OpVariable %_ptr_Function__ptr_PhysicalStorageBuffer_uint Function

The slangc version is v2024.1.21 and spirv-val version is SPIRV-Tools v2024.2 v2024.2.rc1-0-gdd4b663e.

csyonghe commented 2 weeks ago

This is a known problem in spirvopt.

Slang generates valid spirv output if you specify -O0 (which skips spirv-opt invocation), but if you pass the spirv to spirv-opt, it will generate invalid spirv after optimizations. Please file the bug with the spirv-tools repo.

csyonghe commented 2 weeks ago

Closing as it is not related to slang.

jkwak-work commented 2 weeks ago

FYI, this issue is duplicated to https://github.com/shader-slang/slang/issues/3931 @sriramm-nv , do you have any update on this?

BeastLe9enD commented 2 weeks ago

@csyonghe I can confirm this works, thank you!

BeastLe9enD commented 2 weeks ago

@csyonghe have you seen this in the past?

PS C:\Users\BeastLe9enD\Documents\Projects\greek\assets\shaders\virtualized_geometry> slangc.exe -emit-spirv-directly -O3 .\transcode.comp.slang -o transcode.comp.spv | spirv-val.exe transcode.comp.spv error: line 169: OpVariable 107: expected AliasedPointer or RestrictPointer for PhysicalStorageBuffer pointer. %107 = OpVariable %_ptr_Function__ptr_PhysicalStorageBuffer_uint Function

PS C:\Users\BeastLe9enD\Documents\Projects\greek\assets\shaders\virtualized_geometry> slangc.exe -emit-spirv-directly -O0 .\transcode.comp.slang -o transcode.comp.spv | spirv-val.exe transcode.comp.spv error: line 440: OpAccessChain result type (OpTypePointer) does not match the type that results from indexing into the base (OpTypePointer). %118 = OpAccessChain %_ptr_Function__ptr_Function_uint %this %int_0

I see different errors when using -O0 vs -O3, not sure what is the best way to reproduce this in an isolated context.

I added all relevant files in case u are interested: bitstream_writer.slang.txt math.slang.txt bitstream_reader.slang.txt headers.slang.txt transcode.comp.slang.txt

this is the output spirv text: transcode.comp.spv.txt

I tried with v2024.1.21 and v2024.1.22.

csyonghe commented 2 weeks ago

No I haven't. Seems like a bug if -O0 also fails validation.

BeastLe9enD commented 2 weeks ago

@csyonghe okay, should I make a separate issue for this?

csyonghe commented 2 weeks ago

We can take it from here, assuming this is reproducible from the files you attached?

BeastLe9enD commented 2 weeks ago

@csyonghe I tried to reproduce it in a more compact file. With the following shader compiled with slangc -emit-spirv-directly -O0 .\reproduce.slang -o reproduce.slang.spv | spirv-val .\reproduce.slang.spv, I get:

error: line 99: OpAccessChain result type (OpTypePointer) does not match the type that results from indexing into the base <id> (OpTypePointer).
  %61 = OpAccessChain %_ptr_Function__ptr_Function_uint %this %int_0

The shader:

import glsl;

public struct BitstreamWriter {
    private uint* _p;
    private uint _offset;

    public __init(uint* p, uint offset) {
        _p = p;
        _offset = offset;
    }

    [mutating]
    public func write_bits(num_bits: uint, value: uint) {
        let bit_idx = _offset & 31u;
        let idx = _offset >> 5u;

        _offset += num_bits;

        atomicOr(_p[idx], value << bit_idx);
        if (bit_idx + num_bits > 32u) {
            atomicOr(_p[idx + 1], value >> (32u - bit_idx));
        }
    }
}

struct Constants {
    uint* p;
}

[[vk::push_constant]] Constants constants;

[shader("compute")]
[numthreads(1, 1, 1)]
void main(uint dtid: SV_DispatchThreadID) {
    var writer = BitstreamWriter(constants.p, dtid * 5);
    writer.write_bits(5, dtid.x);
}

this is the resulting spirv assembly:

; SPIR-V
; Version: 1.5
; Generator: Khronos; 40
; Bound: 82
; Schema: 0
               OpCapability VariablePointers
               OpCapability PhysicalStorageBufferAddresses
               OpCapability Shader
               OpExtension "SPV_KHR_variable_pointers"
               OpExtension "SPV_KHR_physical_storage_buffer"
               OpMemoryModel PhysicalStorageBuffer64 GLSL450
               OpEntryPoint GLCompute %main "main" %constants %gl_GlobalInvocationID
               OpExecutionMode %main LocalSize 1 1 1
               OpSource Slang 1
               OpName %BitstreamWriter "BitstreamWriter"
               OpMemberName %BitstreamWriter 0 "_p"
               OpMemberName %BitstreamWriter 1 "_offset"
               OpName %writer "writer"
               OpName %writer "writer"
               OpName %Constants_std140 "Constants_std140"
               OpMemberName %Constants_std140 0 "p"
               OpName %constants "constants"
               OpName %constants "constants"
               OpName %p "p"
               OpName %offset "offset"
               OpName %BitstreamWriter__init "BitstreamWriter.$init"
               OpName %this "this"
               OpName %num_bits "num_bits"
               OpName %value "value"
               OpName %BitstreamWriter_write_bits "BitstreamWriter.write_bits"
               OpName %main "main"
               OpDecorate %_ptr_PhysicalStorageBuffer_uint ArrayStride 4
               OpMemberDecorate %BitstreamWriter 0 Offset 0
               OpMemberDecorate %BitstreamWriter 1 Offset 8
               OpDecorate %gl_GlobalInvocationID BuiltIn GlobalInvocationId
               OpDecorate %Constants_std140 Block
               OpMemberDecorate %Constants_std140 0 Offset 0
               OpDecorate %p Aliased
       %void = OpTypeVoid
          %3 = OpTypeFunction %void
       %uint = OpTypeInt 32 0
%_ptr_PhysicalStorageBuffer_uint = OpTypePointer PhysicalStorageBuffer %uint
%BitstreamWriter = OpTypeStruct %_ptr_PhysicalStorageBuffer_uint %uint
%_ptr_Function_BitstreamWriter = OpTypePointer Function %BitstreamWriter
     %v3uint = OpTypeVector %uint 3
%_ptr_Input_v3uint = OpTypePointer Input %v3uint
%Constants_std140 = OpTypeStruct %_ptr_PhysicalStorageBuffer_uint
%_ptr_PushConstant_Constants_std140 = OpTypePointer PushConstant %Constants_std140
        %int = OpTypeInt 32 1
      %int_0 = OpConstant %int 0
%_ptr_PushConstant__ptr_PhysicalStorageBuffer_uint = OpTypePointer PushConstant %_ptr_PhysicalStorageBuffer_uint
     %uint_5 = OpConstant %uint 5
         %27 = OpTypeFunction %BitstreamWriter %_ptr_PhysicalStorageBuffer_uint %uint
%_ptr_Function__ptr_PhysicalStorageBuffer_uint = OpTypePointer Function %_ptr_PhysicalStorageBuffer_uint
      %int_1 = OpConstant %int 1
%_ptr_Function_uint = OpTypePointer Function %uint
         %44 = OpTypeFunction %void %_ptr_Function_BitstreamWriter %uint %uint
    %uint_31 = OpConstant %uint 31
%_ptr_Function__ptr_Function_uint = OpTypePointer Function %_ptr_Function_uint
     %uint_1 = OpConstant %uint 1
    %uint_64 = OpConstant %uint 64
       %bool = OpTypeBool
    %uint_32 = OpConstant %uint 32
%gl_GlobalInvocationID = OpVariable %_ptr_Input_v3uint Input
  %constants = OpVariable %_ptr_PushConstant_Constants_std140 PushConstant
       %main = OpFunction %void None %3
          %4 = OpLabel
     %writer = OpVariable %_ptr_Function_BitstreamWriter Function
         %11 = OpLoad %v3uint %gl_GlobalInvocationID
         %14 = OpCompositeExtract %uint %11 0
         %21 = OpAccessChain %_ptr_PushConstant__ptr_PhysicalStorageBuffer_uint %constants %int_0
         %22 = OpLoad %_ptr_PhysicalStorageBuffer_uint %21
         %23 = OpIMul %uint %14 %uint_5
         %25 = OpFunctionCall %BitstreamWriter %BitstreamWriter__init %22 %23
               OpStore %writer %25
         %42 = OpFunctionCall %void %BitstreamWriter_write_bits %writer %uint_5 %14
               OpReturn
               OpFunctionEnd
%BitstreamWriter__init = OpFunction %BitstreamWriter None %27
          %p = OpFunctionParameter %_ptr_PhysicalStorageBuffer_uint
     %offset = OpFunctionParameter %uint
         %30 = OpLabel
         %31 = OpVariable %_ptr_Function_BitstreamWriter Function
         %33 = OpAccessChain %_ptr_Function__ptr_PhysicalStorageBuffer_uint %31 %int_0
               OpStore %33 %p
         %37 = OpAccessChain %_ptr_Function_uint %31 %int_1
               OpStore %37 %offset
         %39 = OpLoad %BitstreamWriter %31
               OpReturnValue %39
               OpFunctionEnd
%BitstreamWriter_write_bits = OpFunction %void None %44
       %this = OpFunctionParameter %_ptr_Function_BitstreamWriter
   %num_bits = OpFunctionParameter %uint
      %value = OpFunctionParameter %uint
         %48 = OpLabel
         %51 = OpAccessChain %_ptr_Function_uint %this %int_1
         %52 = OpLoad %uint %51
         %53 = OpBitwiseAnd %uint %52 %uint_31
         %55 = OpLoad %uint %51
         %56 = OpShiftRightLogical %uint %55 %uint_5
         %57 = OpLoad %uint %51
         %58 = OpIAdd %uint %57 %num_bits
               OpStore %51 %58
         %61 = OpAccessChain %_ptr_Function__ptr_Function_uint %this %int_0
         %62 = OpLoad %_ptr_PhysicalStorageBuffer_uint %61
         %63 = OpPtrAccessChain %_ptr_PhysicalStorageBuffer_uint %62 %56
         %64 = OpShiftLeftLogical %uint %value %53
         %65 = OpAtomicOr %uint %63 %uint_1 %uint_64 %64
         %68 = OpIAdd %uint %53 %num_bits
         %70 = OpUGreaterThan %bool %68 %uint_32
               OpSelectionMerge %50 None
               OpBranchConditional %70 %49 %50
         %49 = OpLabel
         %73 = OpIAdd %uint %56 %uint_1
         %74 = OpLoad %_ptr_PhysicalStorageBuffer_uint %61
         %75 = OpPtrAccessChain %_ptr_PhysicalStorageBuffer_uint %74 %73
         %76 = OpISub %uint %uint_32 %53
         %77 = OpShiftRightLogical %uint %value %76
         %78 = OpAtomicOr %uint %75 %uint_1 %uint_64 %77
               OpBranch %50
         %50 = OpLabel
               OpReturn
               OpFunctionEnd
BeastLe9enD commented 2 weeks ago

by the way, if I comment out the atomicOr instructions, it does not appear anymore. If using InterlockedOr instead of glsl's atomicOr, it still appears.