[clang] Mechanically port bulk of x86 builtins to TableGen (PR #120831)
Chandler Carruth via cfe-commits
cfe-commits at lists.llvm.org
Sat Dec 21 15:39:49 PST 2024
chandlerc wrote:
> > * Systematically using `Oi` instead of `LLi` for the type `long long int`. The `.def` file uses a mixture of `Oi` and `LLi`. I chose the shorter encoding.
>
> The mixture use of `Oi` and `LLi` is a mess, but `Oi` has different meaning for OpenCL targets. I think we should not change `LLi` to `Oi`. I think a lot `Oi` can be replaced with `LLi`, but I cannot tell which are required with a quick look.
I do understand that, but I'm actually a bit more confident that these changes are correct... Or at least not a regression.
Specifically, the x86 intrinsic builtins use `Oi` *very* consistently for "quad word" (64-bit) vector operations from SSE through AVX2. For example `__builtin_ia32_psllqi256` uses `V4Oi`. This seems quite intentional, so I was very hesitant to reverse it.
The places where `LLi` has crept in are:
- 1 or 2 very isolated cases I'll list explicitly below
- Some AVX-512 and SHA512 intrinsics. This seems like a mistake as they're also using it for "double word" (64-bit) vector elements, the exact same element size that uses `Oi` elsewhere. And some actually *do* use `Oi` with the same feature (`avx10.2-256`), so I couldn't see any pattern that seemed to indicate a critical distinction...
I think the AVX-512 stuff was just added without realizing the historical use of `Oi` here?
The only examples outside of AVX-512 and SHA512 I could find:
- `__builtin_ia32_rdpru`
- `__emul` and `__emulu` (MS intrinsics)
- `__readfsqword` and `__readgsqword` (also MS intrinsics)
I can understand that these don't make lots of sense in OpenCL, but they also seem very unlikely to show up or do the wrong thing here, when getting the same type that OpenCL uses for 64-bit vector elements on x86?
The reason I'd like to consolidate here is that preserving this distinction would add complexity to the tablegen code, and at least *seems* like it may be propagating more of a mistake than a careful choice in AVX-512 (especially given the lack of test coverage that errors with the change as is).
https://github.com/llvm/llvm-project/pull/120831
More information about the cfe-commits
mailing list