[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 10 14:08:08 PDT 2022


rjmccall added a comment.

In D133668#3847807 <https://reviews.llvm.org/D133668#3847807>, @beanz wrote:

> In D133668#3847163 <https://reviews.llvm.org/D133668#3847163>, @rjmccall wrote:
>
>> Sure, but it's extremely easy to unpromote that arithmetic for most operators, and I'm sure LLVM already has a pass that will do that.
>
> Okay... but HLSL explicitly doesn't promote. Having the compiler rely on an optimization to generate correct code is undesirable. Especially since we want debug builds to behave correctly.
>
>> Alternatively, if you're worried about your ability to unpromote, and since you're breaking strict conformance anyway, have you considered just removing the initial promotion to `int` from the usual arithmetic conversions?  I'm pretty sure the rest of the rules hang together just fine if you do.  Then you have a uniform rule that permits easier vectorization for all small integer types rather than being sensitive specifically to using the `int16_t` typedef.
>
> HLSL isn't conformant to C or C++. One of the big areas that things get wonky is that every `int` is actually an implicit SIMD vector of a hardware-determined size.

But that's purely on the implementation level, right?  Everything is implicitly vectorized and you're just specifying the computation of a single lane, but as far as that lane-wise computation is concerned, you just have expressions which produce scalar values.

> We could disable promotion for HLSL. That is what we do in DXC. Part of what made me think `BitInt` was a better solution is that HLSL doesn't have the `short` keyword at all. The only 16-bit int types we support are the `[u]int16_t` explicit-sized typedefs. If you think disabling promotion under the HLSL language mode is a better approach, we can do that instead.

If you don't otherwise have 16-bit (or 8-bit?) types, and it's the type behavior you want, I'm fine with you just using `_BitInt`.  I just want to make sure I understand the situation well enough to feel confident that you've considered the alternatives.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133668/new/

https://reviews.llvm.org/D133668



More information about the cfe-commits mailing list