`DxbcConverter` can corrupt integer Immediate Constant Buffer values (x86)

TL;DR: It is not safe to assume that integer bit-patterns will be preserved when reinterpret-casting to floating-point and back again. DxbcConverter does integer → float → integer reinterpret-casting, causing certain integer values to be silently corrupted. Attempted fixes either causes WARP to crash, or delays the bit-pattern corruption until DXIL is consumed by WARP.

Issue

Test environment: Windows 10 + D3D12 + x86.

Given the following HLSL:

RWByteAddressBuffer b : register(u0, space0);

[numthreads(1, 1, 1)]
void main() {
  {
    for(uint i = 0u; (i < 3u); i = (i + 1u)) {
      const uint icb[3] = {0xffbfffcau, 0x09909909u, 1u};
      b.Store((4u * min(i, 2u)), asuint(icb[min(i, 2u)]));
    }
  }
  return;
}

We compile this with FXC (fxc.exe test.hlsl -Tcs_5_1 /Lx), which gives the following DXBC:

cs_5_1
dcl_globalFlags refactoringAllowed
dcl_immediateConstantBuffer { { 0xffbfffca, 0x00000000, 0x00000000, 0x00000000},
                              { 0x09909909, 0x00000000, 0x00000000, 0x00000000},
                              { 0x00000001, 0x00000000, 0x00000000, 0x00000000} }
dcl_uav_raw U0[0:0], space=0
dcl_temps 1
dcl_thread_group 1, 1, 1
mov r0.x, l(0x00000000)
loop
  uge r0.y, r0.x, l(0x00000003)
  breakc_nz r0.y
  umin r0.y, r0.x, l(0x00000002)
  ishl r0.z, r0.y, l(0x00000002)
  store_raw U0[0].x, r0.z, icb[r0.y + 0].x
  iadd r0.x, r0.x, l(0x00000001)
endloop
ret

Observe that the 0xffbfffca constant in DXBC matches the value in the HLSL program. Everything (so far) is good.

After ensuring that the shader cache (<home>\AppData\Local\D3DSCache) is cleared, we then use this shader to build our D3D12 compute pipeline, which internally invokes the DxbcConverter to convert this DXBC to DXIL.

Inside DxbcConverter::ConvertInstructions() we have the following logic to convert immediate constant buffers:

https://github.com/microsoft/DirectXShaderCompiler/blob/f9bc4f598b420ff211e82523580d8481caac8903/projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp#L2125-L2135

Observe the (float*) cast is reinterpreting the integer ICB data as a LLVM constant float array.

Later, in Bitcode\Writer\BitcodeWriter.cpp, we fetch this the value as a float by calling ConstantDataSequential::getElementAsFloat(), reinterpret it back to uint32_t, then append it to the Record:

https://github.com/microsoft/DirectXShaderCompiler/blob/f9bc4f598b420ff211e82523580d8481caac8903/lib/Bitcode/Writer/BitcodeWriter.cpp#L1519-L1523

The value 0xffbfffca survives the long journey, until the call getElementAsFloat() - upon which, the value is loaded and returned as a float with the bit-pattern 0xffffffca - a u32 difference of 0x400000.

The cdecl calling convention requires this float to be returned in the first floating point register, and the bit-pattern of 0xffbfffca represents a x86 signalling-NaN. If fp-execeptions are disabled, then when this signalling-NaN is loaded by the fld instruction, it is silently converted to a quiet-NaN, and the value's 22nd bit is flipped. The bit-pattern of the float returned by getElementAsFloat() is now 0xffffffca.

0xffffffca is then written out into the DXIL bitcode, and our shader executes with the incorrect constant value.

More details about this kind of bit-flipping problem can found at crbug.com/tint/1714.

---

Potential fix

I had a go at trying to fix the DXBC converter, by changing the m_pIcbGV variable to be an array of u32. So instead of:

https://github.com/microsoft/DirectXShaderCompiler/blob/f9bc4f598b420ff211e82523580d8481caac8903/projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp#L2130

We replace with:

llvm::Constant *pIcbData = ConstantDataArray::get(m_Ctx, ArrayRef<uint32_t>((uint32_t*)Inst.m_CustomData.pData, Size));

And we also have to fix up the cast on the load, so instead of:

https://github.com/microsoft/DirectXShaderCompiler/blob/f9bc4f598b420ff211e82523580d8481caac8903/projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp#L6077

We replace with:

Value *pValue = CastDxbcValue(pLoad, CompType::getU32(), ValueType);

Doing so appears to fix the issue when testing with a Nvidia 2080 Ti. Yay!

However with this fix, the WARP device now crashes with this emitted DXIL:

>	d3d10warp.dll!llvm::APFloat::bitcastToAPInt(void)	Unknown
 	d3d10warp.dll!llvm::APFloat::convertToFloat(void)	Unknown
 	d3d10warp.dll!CShaderInfo::CompileShaderDXIL(char const *,unsigned int,bool)	Unknown
 	d3d10warp.dll!UMShader::Init(unsigned int const *,struct D3D12DDIARG_STAGE_IO_SIGNATURES const *,struct D3D12DDIARG_TESSELLATION_IO_SIGNATURES const *,struct D3D11DDIARG_CREATEGEOMETRYSHADERWITHSTREAMOUTPUT const *,enum D3D12DDI_CREATE_SHADER_FLAGS)	Unknown
 	d3d10warp.dll!UMDevice::CreateComputeShaderInternal(struct D3D10DDI_HDEVICE,unsigned int const *,struct D3D12DDIARG_STAGE_IO_SIGNATURES const *,struct D3D10DDI_HSHADER,enum D3D12DDI_CREATE_SHADER_FLAGS)	Unknown
 	d3d10warp.dll!UMDevice::CreateComputeShader12_0010(struct D3D10DDI_HDEVICE,struct D3D12DDIARG_CREATE_SHADER_0010 const *,struct D3D10DDI_HSHADER)	Unknown
 	D3D12Core.dll!TableFunctionTraits<0>::Detail::InvokerImpl<struct TableFunctionTraitsImpl<0>::FunctionTraits<41,19,void>,void,void,struct D3D10DDI_HDEVICE,struct D3D12DDIARG_CREATE_SHADER_0026 const *,struct D3D10DDI_HSHADER>::Call(class CDevice *,struct D3D10DDI_HDEVICE,struct D3D12DDIARG_CREATE_SHADER_0026 const *,struct D3D10DDI_HSHADER)	Unknown
 	D3D12Core.dll!CShader::FinalConstruct(struct CShader::TConstructorArgs const &)	Unknown
 	D3D12Core.dll!CLayeredObject<class CShader>::CreateInstance(struct CShader::TConstructorArgs const &,void *,void *,struct _GUID const &,void * *)	Unknown
 	D3D12Core.dll!CDevice::CreateLayeredChild(unsigned int,void const *,unsigned long,struct ID3D12LayeredUseCounted *,struct _GUID const &,void * *)	Unknown
 	D3D12Core.dll!CBridgeImpl<struct ID3D12LayeredDevice,struct ID3D12LayeredDevice,class CLayeredObject<class CDevice> >::CreateLayeredChild(unsigned int,void const *,unsigned long,struct ID3D12LayeredUseCounted *,struct _GUID const &,void * *)	Unknown
 	D3D12Core.dll!CBridgeImpl<struct ID3D12LayeredDevice,struct ID3D12LayeredDevice,class CLayeredObject<class NDXGI::CDevice> >::CreateLayeredChild(unsigned int,void const *,unsigned long,struct ID3D12LayeredUseCounted *,struct _GUID const &,void * *)	Unknown
 	D3D12.dll!NOutermost::CDevice::CreateLayeredChild(unsigned int,void const *,unsigned long,struct ID3D12LayeredUseCounted *,struct _GUID const &,void * *)	Unknown
 	D3D12Core.dll!CDevice::CreateShader(struct ID3D12ShaderBytecode *,struct ID3D12RootSignature *,enum D3D12_PIPELINE_STATE_FLAGS,bool,struct SPipelineLibraryReference const &)	Unknown
 	D3D12Core.dll!CDevice::CreatePipelineStateImpl(struct CD3DX12_PIPELINE_STATE_STREAM2 &,struct SPipelineLibraryReference const &,struct _GUID const &,void * *)	Unknown
 	D3D12Core.dll!CDevice::CreateComputePipelineState(struct D3D12_COMPUTE_PIPELINE_STATE_DESC const *,struct _GUID const &,void * *)	Unknown

Based on some poking around with the disassembly, it looks like WARP assumes that the ICB is always an array of floats, and is not checking the type of the "dx.icb" global variable. The name of the variable doesn't seem to matter, so maybe this assumption is on any data in the DXIL::kImmediateCBufferAddrSpace address space?

Given that WARP crashes, I'm concerned that other drivers might also misbehave with ICBs being emitted as integers.

---

Super risky / hacky but minimal work-around

If there's a contract somewhere that states that the ICB must be a float array (I couldn't find one), then a hacky work-around for this precise issue is to change BitcodeWriter to use getElementPointer() instead of getElementAsFloat(), avoiding the reintrepret-cast to and from float.

Instead of:

https://github.com/microsoft/DirectXShaderCompiler/blob/f9bc4f598b420ff211e82523580d8481caac8903/lib/Bitcode/Writer/BitcodeWriter.cpp#L1519-L1523

We do something like:

for (unsigned i = 0, e = CDS->getNumElements(); i != e; ++i) {
  uint32_t I = *reinterpret_cast<const uint32_t*>(CDS->getElementPointer(i));
  Record.push_back(I);
}

Note: you'll also need to make getElementPointer() public. With this, the DXIL bits are preserved, I get the correct output with my Nvidia GPU, WARP does not crash, but:

1. This just so happens to work for this case. Anything in the DxbcConverter, or any consumer of the emitted DXIL, that attempts to load these values as float are likely to run into the signalling -> quiet NaN conversion issue again. This will be a game of whack-a-mole.
2. While the WARP device doesn't crash with this approach, it does appear to load these ICB constants as float and we get our old friend 0xffffffca.
3. The DXIL textual printer also appears to load the value as a float, printing the wrong value.

Given the above, I would discourage this as a fix - but if the ICB type cannot be changed, this might be the only way to fix this issue without jumping through hoops at the HLSL level.

Start with the cited immediate-constant-buffer handling in projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp and serialization in lib/Bitcode/Writer/BitcodeWriter.cpp. Trace the supplied HLSL reproducer from DXBC conversion through DXIL bitcode emission, comparing the integer bit patterns at each step. Done means the x86 D3D12 reproducer preserves the constants without corruption, while Nvidia and WARP avoid the reported incorrect output or crash.