[PATCH] D133634: [clang] Allow vector of BitInt

Mon Sep 12 06:23:17 PDT 2022

aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a subscriber: erichkeane.
aaron.ballman added a comment.

In D133634#3783160 <https://reviews.llvm.org/D133634#3783160>, @python3kgae wrote:

> In D133634#3782999 <https://reviews.llvm.org/D133634#3782999>, @xbolva00 wrote:
>
>> Missing IR tests. No issues in codegen?
>>
>> Very poor description. Why it was disabled? Why we can enable it now?
>
> I'm not sure why it is disabled.
> I guess it is never enabled and just assert in ASTContext::getExtVectorType before.
> Then https://github.com/llvm/llvm-project/commit/c9edf843fcf954132271214445857498fb47bb72 make it an error instead of assert.

It was disabled because it wasn't intentionally enabled -- we've not thought through the best way to expose vectors of _BitInt yet and we didn't want anyone to get tied into an accidental ABI by leaving them exposed.

> I'm enabling it for HLSL to use _BitInt(16) as 16bit int at https://reviews.llvm.org/D133668
> This PR only makes sure it does not fail at AST level.
> https://reviews.llvm.org/D133668 will fix the fail in Mangling when codeGen.

I think this requires more consideration. I think we *want* to enable vectors of `_BitInt` types, but I think we need a more full-featured plan for supporting them. For example, do we want to allow a vector of `_BitInt(5)` objects, or do we want to support only powers-of-two sized `_BitInt`s? How do we ensure ABI compatibility with GCC for these vector types? That sort of thing.

CC @erichkeane who worked on the initial `_ExtInt` implementation in case he's got additional thoughts.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133634