QuadReadAcross(bool) Fails to Compile about directxshadercompiler HOT 5 OPEN

I did a quick investigation here. There is a silly bug in DXC's overload scoring which is probably not easy to fix without side-effects.

A distilled demonstration is:

void f(int I);
void f(float F);

void c(bool B) {
  f(B); // error call to `f` is ambiguous
}

Basically the overload scoring doesn't provide a sane score to conversions from bool. It should prefer bool->int over bool->float.

This seems to cause something funky when we add the overloads to the AST which I still don't fully understand, but somehow we come out only adding the float overload, so DXC resolves to that. All of this code is a big magic-voodoo.

from directxshadercompiler.

The example Chris showed pertains to how user-function overloads are resolved and does represent a real issue. Intrinsics are resolved a bit differently. The QuadReadAcrossX() overload is determined by the pseudo-type indicated here as is interpreted in Sema here. Because it is specified as numeric it doesn't include bool. Its close cousin WaveReadLaneAt uses any instead of numeric, which includes bool. Doing the same thing correctly produces dx.op.quadOp.i1. Unfortunately, the validator identifies this as an invalid overload. That means to fix this, we'd need to either determine that all relevant hardware can handle this anyway, which is possible, or change it in a new shader model.

from directxshadercompiler.

One note I have for @rnvannatta is that the description says that this fails to compile. It doesn't fail, it just uses a float overload. that's weird and probably not intended, but I wanted to confirm that we were considering the same issue: https://godbolt.org/z/GqKh9341d

from directxshadercompiler.

Yup you're correct. It's just using the wrong overload.

Sorry, I always compile with -Werror and just saw lots of compiler noise.

from directxshadercompiler.

@pow2clk, I think the example I pasted highlights exactly the bug that results in this behavior being wrong.

Because of a bug in how SemaHLSL scores Boolean conversions, MatchArguments returns “true” when evaluating the float overload, which signifies that the overload is an exact match. This causes the custom overload handling for built in functions to only provide one overload candidate in the overload set (the float one). See the code here: https://github.com/microsoft/DirectXShaderCompiler/blob/main/tools/clang/lib/Sema/SemaHLSL.cpp#L4936

We can talk about whether this specific case is a bug or not, based on what is set in gen_intrin_main, but that’s not the main problem here. The main problem is that we incorrectly score the overload, and that causes problems with other intrinsics too. Like WaveActiveSum, which shouldn’t have a bool overload, and definitely shouldn’t resolve to a float one.

If we didn’t incorrectly score the overload, rather than populating a single overload candidate, we would populate a full candidate set, and the overload would fail to resolve producing an error. That’s probably the intended behavior here, except for the bug in MatchArguments that says a book argument is an exact match for a float parameter (which is obviously a bug).

from directxshadercompiler.