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

2022-10-18 Thread Xiang Li via Phabricator via cfe-commits
python3kgae updated this revision to Diff 468617. python3kgae added a comment. Switch back to short and disable integer promote for hlsl. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133668/new/ https://reviews.llvm.org/D133668 Files: clang/lib

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

2022-10-10 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. If you have `char`, would you want it to promote? Because turning `char` to `_BitInt(8)` is breaking with C on other grounds (like aliasing), for better or worse. So if you just don't want promotion, maybe you really should just disable promotion. Repository: rG

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

2022-10-10 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. In D133668#3847871 , @rjmccall wrote: > 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 compu

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

2022-10-10 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D133668#3847807 , @beanz wrote: > In D133668#3847163 , @rjmccall > wrote: > >> Sure, but it's extremely easy to unpromote that arithmetic for most >> operators, and I'm sure LLVM alr

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

2022-10-10 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. In 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 t

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

2022-10-10 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. 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. The only thing that blocks unpromotion is when you feed a promoted result into something that's sensitive to working on a wider value

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

2022-10-09 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. Avoiding argument promotion is one part of what we need, but not all of it. For example if you take this trial code: const RWBuffer In; RWBuffer Out; [numthreads(1,1,1)] void main(uint GI : SV_GroupIndex) { Out[GI] = In[GI].x + In[GI].y; } Following C rul

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

2022-10-09 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. If your goal is just to pass 16-bit types in some specific way without promotion, you can just do that in the ABI. C only requires `short` arguments to be promoted to `int` in variadic or unprototyped positions, and it's not legal under the C type compatibility rules

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

2022-10-07 Thread Xiang Li via Phabricator via cfe-commits
python3kgae added a comment. Hi, @rjmccall and @efriedma, Could you take a look at this PR? Or should I find someone else to review it? Thanks Xiang Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133668/new/ https://reviews.llvm.org/D133668 __

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

2022-09-28 Thread Xiang Li via Phabricator via cfe-commits
python3kgae added a comment. Gentle ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133668/new/ https://reviews.llvm.org/D133668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

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

2022-09-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. LGTM, but please wait a bit to land for the codegen reviewers to weigh in on those tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133668/new/ https://reviews.llvm.org/D133668 __

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

2022-09-19 Thread Xiang Li via Phabricator via cfe-commits
python3kgae updated this revision to Diff 461308. python3kgae added a comment. Limit max bitint width to 64 for HLSL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133668/new/ https://reviews.llvm.org/D133668 Files: clang/lib/Basic/Targets/Direc

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

2022-09-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: rjmccall, efriedma. aaron.ballman added a comment. Adding some codegen reviewers for awareness. Comment at: clang/lib/Basic/Targets/DirectX.h:66 + bool hasBitIntType() const override { return true; } bool hasFeature(StringRef Feature) const

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

2022-09-19 Thread Xiang Li via Phabricator via cfe-commits
python3kgae updated this revision to Diff 461251. python3kgae added a comment. Rebase and update test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133668/new/ https://reviews.llvm.org/D133668 Files: clang/lib/Basic/Targets/DirectX.h clang/li

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

2022-09-14 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added inline comments. Comment at: clang/lib/Basic/Targets/DirectX.h:66 + bool hasBitIntType() const override { return true; } bool hasFeature(StringRef Feature) const override { aaron.ballman wrote: > This change requires more testing/thought, IMO --

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

2022-09-12 Thread Xiang Li via Phabricator via cfe-commits
python3kgae added a comment. In D133668#3784636 , @python3kgae wrote: > In D133668#3784607 , @aaron.ballman > wrote: > >> In D133668#3783975 , @beanz wrote: >> >>> In D1

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

2022-09-12 Thread Xiang Li via Phabricator via cfe-commits
python3kgae added a comment. In D133668#3784607 , @aaron.ballman wrote: > In D133668#3783975 , @beanz wrote: > >> In D133668#3783489 , >> @aaron.ballman wrote: >> >>> Ok

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

2022-09-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D133668#3783975 , @beanz wrote: > In D133668#3783489 , @aaron.ballman > wrote: > >> Okay, that's good to know! If you don't intend to *ever* conform to the >> standard in this a

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

2022-09-12 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. In D133668#3783489 , @aaron.ballman wrote: > Okay, that's good to know! If you don't intend to *ever* conform to the > standard in this area, then I think this approach is very reasonable. But you > should know up front that you'

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

2022-09-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D133668#3783090 , @beanz wrote: > HLSL deviates from C here. HLSL doesn't actually have `short` (although I'm > actually not sure we should disable it in Clang). We do have `int16_t`, but > we don't promote `int16_t` to

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

2022-09-11 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. HLSL deviates from C here. HLSL doesn't actually have `short` (although I'm actually not sure we should disable it in Clang). We do have `int16_t`, but we don't promote `int16_t` to `int`. We discussed changing codegen to disable promotion for HLSL, but it seemed more str

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

2022-09-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Drive-by comment before I get into the review: does HLSL intend to follow the standard in terms of behavior of intN_t? If yes, then this doesn't follow the behavior allowed by the standard or the direction WG14 chose. We discussed https://www.open-std.org/jtc1/sc2

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

2022-09-11 Thread Xiang Li via Phabricator via cfe-commits
python3kgae created this revision. python3kgae added reviewers: beanz, pow2clk, Anastasia, aaron.ballman, bogner. Herald added a project: All. python3kgae requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. short will be promited to int in Usua