HLSL: EvaluateAttributeAtSample gives error for valid HLSL code.

[danginsburg]

The following shader fails to compile with glslangValidator:

	struct PS_INPUT
	{
		float4 vPositionOs: TEXCOORD0;
	};

	
	float4 MainPs( PS_INPUT i ) : SV_TARGET0
	{
		float4 vTest = float4(0, 0, 0, 0);
		for ( uint nSample = 0; nSample < 4; nSample ++ )
		{
		  vTest.x += EvaluateAttributeAtSample( i.vPositionOs, nSample );
		}
		return vTest;
	} 

glslangValidator -D -V -S frag -E MainPs test.hlsl produces:

ERROR: test.hlsl:12: 'EvaluateAttributeAtSample' : first argument must be an interpolant, or interpolant-array element
ERROR: test.hlsl:12: 'for sub-statement' : Expected
test.hlsl(12): error at column 67, HLSL parsing failed.
ERROR: 3 compilation errors. No code generated.

Compiles correctly with fxc.exe /Tps_5_0 /EMainPs test.hlsl

Microsoft (R) Direct3D Shader Compiler 9.29.952.3111
Copyright (C) Microsoft Corporation 2002-2009. All rights reserved.

H:\dev\source2\main\src\shaders\core\test.hlsl(12,13): warning X3206: implicit truncation of vector type

//
// Generated by Microsoft (R) HLSL Shader Compiler 9.29.952.3111
//
//
//   fxc /Tps_5_0 /EMainPs test.hlsl
//
//
//
// Input signature:
//
// Name                 Index   Mask Register SysValue Format   Used
// -------------------- ----- ------ -------- -------- ------ ------
// TEXCOORD                 0   xyzw        0     NONE  float   x
//
//
// Output signature:
//
// Name                 Index   Mask Register SysValue Format   Used
// -------------------- ----- ------ -------- -------- ------ ------
// SV_TARGET                0   xyzw        0   TARGET  float   xyzw
//
ps_5_0
dcl_globalFlags refactoringAllowed
dcl_input_ps linear v0.x
dcl_output o0.xyzw
dcl_temps 1
mov r0.xy, l(0,0,0,0)
loop
  uge r0.z, r0.y, l(4)
  breakc_nz r0.z
  eval_sample_index r0.z, v0.x, r0.y
  add r0.x, r0.z, r0.x
  iadd r0.y, r0.y, l(1)
endloop
mov o0.x, r0.x
mov o0.yzw, l(0,0,0,0)
ret
// Approximately 11 instruction slots used

[greg-lunarg]

The mechanism that maps members of PS_INPUT onto their true built-in identities somehow got missed. So i.vPositionOs was just looking like a function scope struct member rather than a texcoord input variable.

[greg-lunarg]

There is a reason this doesn't currently work.

There is a general mechanism that @loopdawg created to propagate built-in interpolants from their input structures into the texture operations that use them. It involves a wrapper function around main and inlining from spirv-opt. Unfortunately, this mechanism doesn't quite work for the EvaluateAttributeAtSample operation. That is because while all the SPIR-V texture operations are fine with the values of the texcoords, OpInterpolateAtSample, the SPIR-V operation used to implement EvaluateAttributeAtSample, wants a pointer to the texcoords. Unfortunately, the general mechanism doesn't support this :(

There are two ways to solve this. One way is to get the SPIR-V WG to define new versions of OpInterpolate* to that take values of interpolants rather than pointers.

The other solution is to try to come up with a new mechanism that somehow propagates pointers instead of values. This would be a very invasive change that would likely touch every texture operation.

The idea of destabilizing the entire world to make a fairly marginal operation work seems to go against basic engineering principles. So I am thinking we talk to the SPIR-V workgroup about defining a new set of OpInterpolate* that are as simple to deal with as all of the other texture operations.

[danginsburg]

@greg-lunarg I don't know if I understand the specific issue here, but assuming DXC supports this as-is it seems unlikely we'd be able to convince the SPIR-V WG to create a new SPIR-V extension to make up for glslang internal architecture issues? I also don't know how useful that would be if it would require driver changes since we'd need this to work on today's drivers. I can respect it potentially not making sense to fix this in the HLSL frontend of glslang but unless I'm missing something I don't see how proposing a SPIR-V extension makes sense.

[greg-lunarg]

OK, I think I have a good third solution.

For HLSL, glslang will generate the OpInterpolate* instructions as if they indeed took the value of the interpolant rather than the pointer. Then I will write a new "legalization" pass in spirv-opt that changes the interpolant argument of all OpInterpolate* operations from OpLoad (which they have to be by definition of the HLSL EvaluateAttribute* intrinsics) to the address used by the OpLoad, very likely an OpVariable, possibly an OpAccessChain.

This solution would use the existing glslang interpolant propagation mechanism, so no changes there. Since the fixup would happen as part of legalization, the "invalid" version would never see the light of day. I could probably also rig up some fairly simple code so that the new fixup pass would ONLY be run if OpInterpolate* operations are generated. And there would be no need for new OpInterpolate* operations.

I am fairly certain you will be ok with this approach and will pursue this solution unless you come up with a show-stopper issue.

[greg-lunarg]

If people really had a problem with the "invalid" form of the OpInterpolate* operations, I could create new "internal" versions of the operations that take the new form but are replaced by legalization with the "external" versions.

I am fairly certain this will not be necessary, but we could use it as a fallback position in case there are objections or problems. I will probably bounce this design off the other spirv-opt folks and make sure they don't see any immediate problems.