microsoft / DirectXShaderCompiler

This repo hosts the source for the DirectX Shader Compiler which is based on LLVM/Clang.
Other
2.98k stars 665 forks source link

Incorrect SPIR-V generated when `firstbithigh` is used on a `uint64_t` #4702

Open manon-traverse opened 1 year ago

manon-traverse commented 1 year ago
generated SPIR-V is invalid: GLSL.std.450 FindUMsb: expected all operands to have the same bit width as Result Type
  %137 = OpExtInst %uint %1 FindUMsb %136

note: please file a bug report on https://github.com/Microsoft/DirectXShaderCompiler/issues with source code if possible

Calling this function causes the error when targeting SPIR-V. It works fine when targeting DXIL.

uint clz64(uint64_t v) { return 63 - firstbithigh(v); }

It does work with the following workaround:

uint clz64(uint64_t v) {
    uint2 workaround = uint2(v, v >> 32);

    uint high = firstbithigh(workaround.y);
    if (high != -1) {
        return 31 - high;
    } else {
        return 63 - firstbithigh(workaround.x);
    }
}
godlikepanos commented 1 year ago

FindUMsb and a few similar GLSL derived instructions do not support 64bit (see https://registry.khronos.org/SPIR-V/specs/unified1/GLSL.std.450.html). glslang has (or had) similar issues. I think Khronos should extend the instructions to support 64bit and in the meantime DXC should apply your workaround.

sudonatalie commented 9 months ago

DXC now emits an appropriate error message when firstbithigh is used on a uint64_t. Eventually we should support this, but it's not currently a priority to implement so the workaround in HLSL should be used until then.

int firstbithigh64(uint64_t v) {
    int high = firstbithigh(uint(v >> 32));
    if (high != -1) {
      return high + 32;
    }

    return firstbithigh(uint(v));
}
LukasBanana commented 3 months ago

The fix b7a50ba introduced a new issue at astContext.getTypeSize(argType) == 64 when used on int2 types. This reproduces the issue (see godbolt.org):

// -T vs_6_0 -E VSMain -spirv
int2 Foo;
void VSMain() {
    firstbithigh(Foo);
}
sudonatalie commented 1 month ago

Thanks for catching that @LukasBanana! Submitting a fix now.