slang icon indicating copy to clipboard operation
slang copied to clipboard

Constant propogation can incorrectly perform sign extension

Open expipiplus1 opened this issue 2 years ago • 0 comments

testcase:

//TEST(compute):COMPARE_COMPUTE(filecheck-buffer=CHECK):-vulkan -output-using-type

// CHECK: 9223372036854775808
// CHECK: 1
// CHECK: 0
// CHECK: 18446744073709551615

// This tests exhibits a bug in constant folding in which the right shift
// operator unconditionally performs sign-extension.

//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=8):out,name=outputBuffer
RWStructuredBuffer<uint64_t> outputBuffer;

[numthreads(1, 1, 1)]
void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
{
    const uint64_t topBitSet = 0x8000000000000000ull;
    const uint64_t bottomBitSet = topBitSet >> 63; // This actually ends up sign-extending to set all the bits
    const uint64_t noBitsSet = topBitSet >> 64; // This exhibits undefined behaviour, as the compiler shifts right by at least the number of bits in the first operand
    const uint64_t allBitsSet = int64_t(topBitSet) >> 63;
    outputBuffer[0] = topBitSet;
    outputBuffer[1] = bottomBitSet;
    outputBuffer[2] = noBitsSet;
    outputBuffer[3] = allBitsSet;
}

The code which caught my eye:

https://github.com/shader-slang/slang/blob/188482e83dc4f7dc4c940214033dd95518b9d18c/source/slang/slang-ir-sccp.cpp#L682-L686

This code should check the type of the first operand and decide to sign-extend or not.

Additionally, it and the left shift version should take care not to shift by more than 64 bits as this is UB

Additionally, the above test case fails for the CPU target

expipiplus1 avatar Feb 27 '24 20:02 expipiplus1