Implement correct interpolation semantics for int/uint

[chaoticbob]

Hopefully this isn't a duplicate, I couldn't find it via search.

This is several issues wrapped into one.

1) No error issued for HLSL when uint/int passed between shader stages. If I attempt to compile this GLSL shader, I get an error:

#version 450
#extension GL_ARB_separate_shader_objects : enable
#extension GL_ARB_shading_language_420pack : enable
layout(location = 0) in uint no_interp;
layout(location = 0) out vec4 oColor;
void main()
{
  oColor =  vec4(float(no_interp), 0, 0, 1);
}

ERROR: flat.glsl:5: 'uint' : must be qualified as flat in
ERROR: flat.glsl:5: '' : compilation terminated

However, this HLSL shader compiles through fine, with no error:

struct PSInput {
  uint no_interp;
};
float4 main(PSInput input) : SV_TARGET
{
  return float4(float(input.no_interp), 0, 0, 1);
}

2) Is flat really applied struct wide? When compiling the shader below, glslang indicates that the struct variable should be flat. Does this make sense? Can/should the flat be applied on a per member basis?

#version 450
#extension GL_ARB_separate_shader_objects : enable
#extension GL_ARB_shading_language_420pack : enable
struct MyStuff {
  vec4 colors[4];
  int which;
};
layout(location = 0) in MyStuff no_interp;
layout(location = 0) out vec4 oColor;
void main()
{
  oColor =  no_interp.colors[no_interp.which];
}

ERROR: flat.glsl:20: 'structure' : must be qualified as flat in
ERROR: flat.glsl:20: '' : compilation terminated

Again, the HLSL version compiles through without a problem:

struct MyStuff {
  float4 colors[4];
  int which;
};
struct PSInput {
  MyStuff no_interp;
};
float4 main(PSInput input) : SV_TARGET
{
  return input.no_interp.colors[input.no_interp.which];
}

---

The net result of the HLSL compiles is that the Flat decoration is not applied:

   Decorate 28(input) Location 0
   Decorate 31(@entryPointOutput) Location 0

Adding nointerpolation in HLSL does fix the issue, however it would be nice to get the same error as GLSL in this case.

[johnkslang]

Or, is this a case where it is Vulkan specific, but not HLSL specific?

Or is it difficult to use fxc as as HLSL validator? The glslang HLSL project has been up front about it not being a full HLSL validator. That is a a quite large project on its own, and even more undocumented than accepting valid HLSL.

That said, we generally do most syntactic error checking, and a few semantics (yours is a semantic one). We can add a few critical ones as we go, but we should be clear it's that case, rather than in general a request to do full validation.

[gwihlidal]

There are some rules with HLSL that need to be respected, and I don't believe are currently being handled.

For example:

https://msdn.microsoft.com/en-us/library/windows/desktop/bb509668(v=vs.85).aspx

"When using an int/uint type, the only valid option is nointerpolation."

Can we start by getting int/uint members on input\output structs marked with nointerpolation?

Cheers, Graham

[chaoticbob]

FXC will assume uint vars to be flat even if the nointerpolation modifier isn't applied. This shader:

struct PSInput {
  float interp;
  uint no_interp;
};
float4 main(PSInput input : INPUT) : SV_TARGET
{
  return float4(float(input.no_interp), input.interp, 0, 1);
}

...produces this...

ps_5_1
dcl_globalFlags refactoringAllowed
dcl_input_ps linear v0.x
dcl_input_ps constant v1.x
dcl_output o0.xyzw
utof o0.x, v1.x
mov o0.y, v0.x
mov o0.zw, l(0,0,0,1.000000)
ret 

Seems like the uint value is decorated to be constant? Assuming this means nointerpolation/flat?

The struct version:

struct MyStuff {
  float4 colors[4];
  int which;
};
struct PSInput {
  MyStuff no_interp;
};
float4 main(PSInput input : INPUT) : SV_TARGET
{
  return input.no_interp.colors[input.no_interp.which];
}

...produces this...

ps_5_1
dcl_globalFlags refactoringAllowed
dcl_input_ps linear v0.xyzw
dcl_input_ps linear v1.xyzw
dcl_input_ps linear v2.xyzw
dcl_input_ps linear v3.xyzw
dcl_input_ps constant v4.x
dcl_output o0.xyzw
dcl_temps 1
dcl_indexrange v0.xyzw 4
mov r0.x, v4.x
mov o0.xyzw, v[r0.x + 0].xyzw
ret 

And again, the it seems like the uint value is held constant.

[johnkslang]

Okay, too many issues here, or conflating "semantic checking" with "correct semantics".

Farming off semantic checking and validation to #941.

[johnkslang]

Premature post. Continuing last one... changing the title of this one to implement correct no-interpolation semantics.

Agreed that correct SPIR-V should be generated, as per what's expected here for HLSL in terms of what default interpolation is.

[johnkslang]

This should be fixed now for individual integer inputs going into the fragment stage. (They are forced to flat.)

GLSL doesn't do generic structure member interface stuff. That is intended for blocks and their members, not nested structures. Vulkan reflects that. In this way there is an "off by one" in depth needed between HLSL structures and GLSL blocks. Maybe the loose HLSL input structs should really map to Vulkan input blocks (instead of structs), which would then naturally carry over at least the first level of member decorations. (Vulkan didn't want arbitrary depth in decorations.)

[chaoticbob]

Thanks! I'll give it ago.

[johnkslang]

About structures, I tested that this works with HLSL:

struct PS_INPUT
{
    nointerpolation float4 noInterpIn;
    noperspective float4 noPerspIn;
    linear float4 linearIn;
};

float4 main(PS_INPUT inp : ouou) : SV_TARGET0
{
   return inp.noInterpIn + inp.noPerspIn + inp.linearIn;
}

The SPIR-V contains

                              MemberDecorate 30(PS_INPUT) 0 Flat
                              MemberDecorate 30(PS_INPUT) 1 NoPerspective

And, it is legal by the SPIR-V spec, which says

When applied to structure-type members, the Decorations Noperspective, Flat, Patch, Centroid, and Sample can only be applied to the top-level members of the structure type. (Nested objects' types cannot be structures whose members are decorated with these decorations.)

So, will also make the new integer rule go down a level to do that.

[chaoticbob]

Hi John, just wanted to double check, uint should be flat without the need for an explicit nointerpolation?

[johnkslang]

Yes. fxc actually makes all int stuff be nointerpolation, no matter what you say.

(MS documentation is wrong: it says, say "linear int" is invalid. But, actually, it is valid, and it means "nointerpolation int", hence one reading the docs would never think to implement this.)

Is it not working for some types?

[chaoticbob]

I'm using the test below, my expectation was that no_interp would have Flat applied to it regardless, am I wrong on that?

struct PSInput {
  float interp;
  uint no_interp;
};

float4 main(PSInput input : INPUT) : SV_TARGET
{
  return float4(float(input.no_interp), input.interp, 0, 1);
}

[johnkslang]

Sorry, there is a type cache for sharing structures that needs to be made smarter for this case, will fix soon.

[johnkslang]

Should work now for implicit editing of your "pure" case (it had no appearance of being I/O related, so was missing in an I/O cache). Also, picked up bool and double for implicit Flat treatment.

[chaoticbob]

Hi John, just tested and the fix works. Thanks!

Just one other small thing :) It's looking like the flat decoration of the nested struct member may applied to the wrong member index:

struct MyStuff {
  float4 colors[4];
  int which;
};

struct PSInput {
  MyStuff no_interp;
};

float4 main(PSInput input : INPUT) : SV_TARGET {
  return input.no_interp.colors[input.no_interp.which];
}

Should the OpMemberDecorate be applied to index 1? Or is my understanding wrong on this?

                              MemberName 12(MyStuff) 0  "colors"
                              MemberName 12(MyStuff) 1  "which"
                              Name 13  "PSInput"
                              MemberName 13(PSInput) 0  "no_interp"
                              Name 17  "@main(struct-PSInput-struct-MyStuff-vf4[4]-i111;"
                              Name 16  "input"
                              Name 29  "input"
                              Name 30  "PSInput"
                              MemberName 30(PSInput) 0  "no_interp"
                              Name 32  "input"
                              Name 38  "@entryPointOutput"
                              Name 39  "param"
                              MemberDecorate 30(PSInput) 0 Flat
                              Decorate 32(input) Location 0
                              Decorate 38(@entryPointOutput) Location 0

[johnkslang]

You don't get arbitrary nesting of flat with Vulkan. You get one level.

If a nested struct has stuff in in that must be flat, the whole struct is flat. The SPIR-V is saying the first member of PSInput is flat, because it's a structure that has stuff in it that must be flat.

Will obviously want to know how common this is in real workloads, to have flat-distinctions two-levels deep.

Rant: It's becoming pretty clear that HLSL is designed to be very quickly totally flattened. "struct" is just semantic sugar versus anything actually useful through the compiler stack. (flat things can have a totally different path through hardware than interpolated things, bundling them together in a deeply nested structure is not useful, unless your language just immediately abandons all sense of "struct"). Other languages can keep "struct" down to quite low-level and get benefit from it. But HLSL seems require missing out on all that.

[chaoticbob]

Got it! Thanks for the explanation. I'll see if I can get an idea of how widely it's used. Sorry it's gotten to the point that it's rant worthy

[johnkslang]

Well, the rant was directed at HLSL. :)

The practical point though will be how much more glslang needs to flatten structures. We conditionally do it where needed, to keep the best of both worlds. But, it could end up a different design flattening everything is called for. This depends on real workload usage.