[SPIR-V] Fixed a crash if encounter constant buffer fields with overlapping register assignments

[iOrange]

The issue:
simple vertex shader like so

uniform float4x4 gMVP : register(c0);
uniform float4   gFoo : register(c5);
uniform float4   gBar : register(c5);

float4 main(float4 pos : POSITION) : SV_Position {
    return mul(gMVP, pos * gFoo + gBar);
}

will result in an internal crash

dxc.exe -spirv -T vs_6_2 -E main test.hlsl -Fo test.spirv
Internal compiler error: access violation. Attempted to read from address 0x0000000000000000

Due to LowerTypeVisitor trying to assign offsets to fields without explicit locations.
It'll sort fields first, which will fill the map with the fields first. And since it's using std::map - if there's fields with the same register number - it'll only insert first, other will be left out, resulting nullptrs in the output vector.
We read the content of the vector down the road crashing.

My change fixes the crash and tries to output somewhat useful info about compilation fail.

I hope this helps you in fixing it properly, or you can take it as it is.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[iOrange]

@microsoft-github-policy-service agree

[s-perron]

Looks reasonable to me, but we should add a test to https://github.com/microsoft/DirectXShaderCompiler/tree/68dedee546982f23a47766c20d37d587befb5ed0/tools/clang/test/CodeGenSPIRV.

[s-perron]

/AzurePipelines run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Keenuts]

Thanks for the fix! LGTM, but will require a test before approval/merging 😊

[iOrange]

Sorry for the delay - fixed formatting and added a test

[Keenuts]

Sorry I forgot to check this during the initial review, but seems like we have a behavior divergence between SPIR-V and DXIL. I think what you implemented is correct but need to double check in case that's a known HLSL weirdness.

@spall : is DXIL allowing an overlap expected or a bug?

// RUN: %dxc -T vs_6_6 -E vs_main %s -O3
// RUN: %dxc -T vs_6_6 -E vs_main %s -O3 -spirv

uniform float4x4 gMVP : register(c0);
uniform float4 gFoo : register(c0);

float4 vs_main(float4 pos : POSITION) : SV_Position {
  return mul(gMVP, pos * gFoo);
}

; cbuffer $Globals
; {
;
;   struct hostlayout.$Globals
;   {
;
;       column_major float4x4 gMVP;                   ; Offset:    0
;       float4 gFoo;                                  ; Offset:    0
;
;   } $Globals;                                       ; Offset:    0 Size:    64
;
; }

[Keenuts]

/AzurePipelines run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[iOrange]

Sorry I forgot to check this during the initial review, but seems like we have a behavior divergence between SPIR-V and DXIL. I think what you implemented is correct but need to double check in case that's a known HLSL weirdness.

@spall : is DXIL allowing an overlap expected or a bug?

// RUN: %dxc -T vs_6_6 -E vs_main %s -O3
// RUN: %dxc -T vs_6_6 -E vs_main %s -O3 -spirv

uniform float4x4 gMVP : register(c0);
uniform float4 gFoo : register(c0);

float4 vs_main(float4 pos : POSITION) : SV_Position {
  return mul(gMVP, pos * gFoo);
}

; cbuffer $Globals
; {
;
;   struct hostlayout.$Globals
;   {
;
;       column_major float4x4 gMVP;                   ; Offset:    0
;       float4 gFoo;                                  ; Offset:    0
;
;   } $Globals;                                       ; Offset:    0 Size:    64
;
; }

Hi @Keenuts,

This is the separate DXC issue I've came across - it detects registers overlap, but not if it overlaps with the array. I was going to dive into that one separately.

[tex3d]

Sorry I forgot to check this during the initial review, but seems like we have a behavior divergence between SPIR-V and DXIL. I think what you implemented is correct but need to double check in case that's a known HLSL weirdness.
@spall : is DXIL allowing an overlap expected or a bug?

// RUN: %dxc -T vs_6_6 -E vs_main %s -O3
// RUN: %dxc -T vs_6_6 -E vs_main %s -O3 -spirv

uniform float4x4 gMVP : register(c0);
uniform float4 gFoo : register(c0);

float4 vs_main(float4 pos : POSITION) : SV_Position {
  return mul(gMVP, pos * gFoo);
}

; cbuffer $Globals
; {
;
;   struct hostlayout.$Globals
;   {
;
;       column_major float4x4 gMVP;                   ; Offset:    0
;       float4 gFoo;                                  ; Offset:    0
;
;   } $Globals;                                       ; Offset:    0 Size:    64
;
; }

Hi @Keenuts,

This is the separate DXC issue I've came across - it detects registers overlap, but not if it overlaps with the array. I was going to dive into that one separately.

Yes, this is a DXC bug. Some of the corner cases for register bindings are broken in DXC, and this is one of them.

[Keenuts]

Thanks, LGTM then!

[s-perron]

/AzurePipelines run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).