microsoft / DirectXShaderCompiler

This repo hosts the source for the DirectX Shader Compiler which is based on LLVM/Clang.
Other
3.11k stars 698 forks source link

[SPIR-V] GetAttributeAtVertex does not work with arrays #6259

Open s-perron opened 9 months ago

s-perron commented 9 months ago

Description The SPIR-V code generated by DXC when an array is marked with nointerpolation seems broken. It looks like internally we are inconsistent in how we expand the array into a two dimensional array.

Steps to Reproduce

I tried a number of examples, and they all fail. Here is one example: https://godbolt.org/z/dahbcj3EE.

// RUN: %dxc -T ps_6_0 -E main -fcgl  %s -spirv | FileCheck %s

struct PSInput {
  nointerpolation      float4 fp_h[2]: FPH;
};             

struct PSInput2 {
  float4 fp_h[2]: FPH;
};

float4 main(nointerpolation PSInput2 input) : SV_Target {
  return input.fp_h[1] + GetAttributeAtVertex(input.fp_h[0], 2);
}

Actual Behavior

The spir-v of interest is:

   %PSInput2 = OpTypeStruct %_arr__arr_v4float_uint_2_uint_3
         %35 = OpAccessChain %_ptr_Function_v4float %input %int_0 %uint_0 %int_1
         %36 = OpLoad %v4float %35
         %37 = OpAccessChain %_ptr_Function_v4float %input %int_0 %int_0 %uint_2
         %38 = OpLoad %v4float %37
         %39 = OpFAdd %v4float %36 %38

Note that we have an array of size 3 containing the array of size 2. This means that the order of the access chains should be

%v = OpAccessChain %type %input [[index into the PSInput struct]] [[index for the vertex]] [[index into fg_h array]]

The first access chain looks correct. The provoking vertex is 0, and we want index 1 in the array. However, the second access chain has the indices backwards. I would expect vertex 2 at index 0. This is probably due to the array access being on the input to GetAttributeAtVertex instead of the output. However, if I move [0] to be applied to the return value of GetAttributeAtVertex, there is a compilation error.

There seems to be a design issue here.

Environment

s-perron commented 9 months ago

@ShchchowAMD Do you have any idea how this is suppose to work?

s-perron commented 9 months ago

I checked how the code is generated in glsl. The type for fp_h[2] is correct. The problem is in these lines:

https://github.com/microsoft/DirectXShaderCompiler/blob/c997ea02605d2bc30176cfab14fa205ece2f5af9/tools/clang/lib/SPIRV/SpirvEmitter.cpp#L12151-L12157

The code assumes that is can add the index for GetAttributeAtVertex by appending another index to the end of the access chain, but in this case that is wrong. It needs to place the index before the index into the array, which is already generated.

ShchchowAMD commented 9 months ago

Sorry for the late reply, I'll take a look and give a fix soon. Thanks!