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

[SPIR-V] Intrinsics returning vk::SpirvOpaqueType</*spv::OpTypePointer*/32, don't codegen properly #6978

Open
devshgraphicsprogramming opened this issue Oct 21, 2024 · 1 comment
Labels
bug Bug, regression, crash needs-triage Awaiting triage spirv Work related to SPIR-V

Comments

@devshgraphicsprogramming
Copy link

devshgraphicsprogramming commented Oct 21, 2024

Description

DXC gets mightly confused by Inline SpirV intrinsics.

@s-perron this will probably spoil your fun with Workgroup storage class pointers.

Steps to Reproduce

If we make no temporary image_ptr_t like so it works : https://godbolt.org/z/sEGKrGaao

template<typename T, int32_t Dim, typename ImgT>
[[vk::ext_instruction(/*spv::OpImageTexelPointer*/60)]]
T imageTexelPointer([[vk::ext_reference]] ImgT Image, vector<uint32_t,Dim> coord, uint32_t sample);

...

    spirv::atomicFAddEXT(spirv::imageTexelPointer<float32_t>(velImgF32,coord,0),/*spv::ScopeDevice*/1,/*spv::MemorySemanticsMaskNone*/0,val);

However if we try to make the intrinsic actually return a Spir-V pointer type we run into trouble https://godbolt.org/z/bjjGo5xGM

template<typename T, int32_t Dim, typename ImgT>
[[vk::ext_instruction(/*spv::OpImageTexelPointer*/60)]]
image_ptr_t<T> imageTexelPointer([[vk::ext_reference]] ImgT Image, vector<uint32_t,Dim> coord, uint32_t sample);

...

    spirv::image_ptr_t<float32_t> pTexel = spirv::imageTexelPointer<float32_t>(velImgF32,coord,0);
    spirv::atomicFAddEXT(pTexel,/*spv::ScopeDevice*/1,/*spv::MemorySemanticsMaskNone*/0,val);

Actual Behavior

Insight provided by -Vd https://godbolt.org/z/4z44joGqE

spirv::imageTexelPointer<float32_t>(velImgF32,coord,0);

generates

         %20 = OpImageTexelPointer %_ptr_Image__ptr_Image_float %velImgF32 %19 %uint_0

But what is _ptr_Image__ptr_Image_float you might ask?

%_ptr_Image_float = OpTypePointer Image %float
%_ptr_Image__ptr_Image_float = OpTypePointer Image %_ptr_Image_float

Pointer of Image Storage class to Pointer of Image Storage Class!

The compiler seems to expect that the Inline SPIRV intrinsic always return a non-SPIR-V type T which is supposed to be a variable, so the SPIR-V codegen makes a "pointer to T" declaration.

Its because it slaps an extra layer of pointer on your return type, thats why it worked for the above original "wrong" return type of "just" T. Frankly I am amazed it guessed the storage class right.

This is obviously wrong for an intrinsic you'd want to return an actual SPIR-V pointer. Why would you want to return an inline SPIR-V pointer? Because you don't want DXC's codegen to guess the storage class for you / be in control!

Example: velImgF32 is a Function Storage Class pointer of OpTypeImage %float 3D 2 0 0 2 R32f type, and OpImageTexelPointer produces an Image Storage Class pointer to float out of it.

Environment

  • DXC version: Godbolt trunk
  • Host Operating System <!--- Host operating system and version --->
@devshgraphicsprogramming devshgraphicsprogramming added bug Bug, regression, crash needs-triage Awaiting triage spirv Work related to SPIR-V labels Oct 21, 2024
@damyanp damyanp moved this to For Google in HLSL Triage Oct 22, 2024
@s-perron
Copy link
Collaborator

The problem is that internally DXC lies to itself. When image texel pointers are needed it will generate the OpImageTexelPointer instruction with the pointee type. Then a later pass will create a pointer to that type when it sees the OpImageTexelPointer opcode.

In this case, the compiler generates the pointer type right away, and then the pass will create a pointer to a pointer. The fix is to make sure the code linked below is not run in these cases:

case spv::Op::OpImageTexelPointer: {
const SpirvType *pointerType =
spvContext.getPointerType(resultType, spv::StorageClass::Image);
instr->setResultType(pointerType);
break;

@s-perron s-perron added this to the Next+1 Release milestone Oct 28, 2024
@s-perron s-perron moved this from For Google to Triaged in HLSL Triage Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash needs-triage Awaiting triage spirv Work related to SPIR-V
Projects
Status: New
Status: Triaged
Development

No branches or pull requests

2 participants