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

[BeastLe9enD]

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]

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]

Closing as it is not related to slang.

[BeastLe9enD]

@csyonghe I can confirm this works, thank you!

[BeastLe9enD]

@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]

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

[BeastLe9enD]

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

[csyonghe]

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

[BeastLe9enD]

@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]

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.

[BeastLe9enD]

any progress on this? would be neat as it is a total blocker for me atm

[jkwak-work]

I will see what is going on.

[jkwak-work]

I think this might be a known issue.
If you use [ForceInline] keyword where you use [mutating], you will be able to workaround the problem.
Please let us know if the workaround can unblock you or not.

I will do more investigation and file a new issue if needed.

[BeastLe9enD]

@jkwak-work I can indeed confirm, that if I add [ForceInline] everywhere where I have [mutating], the spirv-val error does no longer appear. Thanks a lot! I think this unblocks me for now, if I'm having further problems, I'll let you know.
Is there already an issue for this known problem you described ? (just out of curiosity)

[jkwak-work]

@jkwak-work I can indeed confirm, that if I add [ForceInline] everywhere where I have [mutating], the spirv-val error does no longer appear. Thanks a lot! I think this unblocks me for now, if I'm having further problems, I'll let you know. Is there already an issue for this known problem you described ? (just out of curiosity)

I am currently trying to understand the nature of the problem.
I initially thought that the result from an atomic operation is overridden by inout type of copy out to this.
But when I looked at the spirv assembly code, it looks more of a problem with the memory address type mismatching.

When [ForceInline] is used, the atomic operation happens directly on the constant buffer memory.
When [ForceInline] is not used, the atomic operation is happening in the intermediate memory space.
I am trying to clarify exactly what memory types are involved in.
I am planning to file an issue if there isn't one already.

[jkwak-work]

Here is a simpler repro case for a future reference.

//TEST:SIMPLE:-target spirv-asm -entry computeMain
//TEST:SIMPLE:-target spirv-asm -entry computeMain -DREPRODUCE
struct BitstreamWriter
{
    uint* p_;

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

    [mutating]
    void write_bits(uint* p)
    {
#if defined(REPRODUCE)
        InterlockedOr(p_[0], 1);
#else
        InterlockedOr(p[0], 1);
#endif
    }
}

struct Constants
{
    uint* p;
}

[[vk::push_constant]] Constants constants;

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

[csyonghe]

By default, [mutating] passes "this" as an inout parameter. Inout parameter is copy-in and copy-out so the compiler can create intermediate variables.

If atomic operations are used through the this parameter, it shall be passed by reference. Putting [__Ref] attribute on the member method will do that. Internally, the compiler also inlines all functions with __ref parameter when generating spirv.

[BeastLe9enD]

By default, [mutating] passes "this" as an inout parameter. Inout parameter is copy-in and copy-out so the compiler can create intermediate variables.

If atomic operations are used through the this parameter, it shall be passed by reference. Putting [__Ref] attribute on the member method will do that. Internally, the compiler also inlines all functions with __ref parameter when generating spirv.

Using [__ref] also fixed the issue. The only thing that is copied tho in my opinion (then using inout) is the uint* value, so shouldn't using [mutating] also work?

[csyonghe]

indeed. I now realize that your original function don't even need [mutating] because it is not modifying anything inside the struct. It is modifying memory pointed to by p, but that is not in the struct itself so mutating is not needed there.

However using mutating and passing this as inout shouldn't result in invalid code.

[BeastLe9enD]

@csyonghe yea I was only using mutating to add to my _offset variable inside the BitstreamReader, this was not related to the atomic operation.

[csyonghe]

With PR #4600 I am no longer seeing any validation errors with -O0.