galios field arithmetic (GF(2^8)) insns:
gf2p8affineinvqb
gf2p8affineqb
gf2p8mulb
Details
- Reviewers
craig.topper - Commits
- rL318993: [x86][icelake]GFNI
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/Support/Host.cpp | ||
---|---|---|
1227 | This needs to be rebased. The current code is here is ordered by bit position and nicely formatted. Please insert this bit in the correct position in the code | |
lib/Target/X86/X86ISelLowering.cpp | ||
19835 | I have trouble believing you need a 2OP_MASKZ. Can't the caller pass the zero vector to the mask version like we do for everything else? The reason we have OP3_MASKZ is because the OP3_MASK uses the passthru input in both the select op and the operation. So we needed a separate intrinsic to define the zero semantics of the select independent of the operation. | |
19922 | Again I think you can pass the zero vector to the masked intrinsic when its called. |
lib/Target/X86/X86InstrAVX512.td | ||
---|---|---|
10028 | This requires BWI in order to make v64i8 a legal type. |
lib/Target/X86/X86ISelLowering.cpp | ||
---|---|---|
19835 | sorry that should have said FMA_MASKZ not OP3_MASKZ. |
lib/Target/X86/X86ISelLowering.cpp | ||
---|---|---|
19835 | interesting |
lib/Target/X86/X86InstrAVX512.td | ||
---|---|---|
10028 | just noted i've nonchalantly added to the test but omitted it here |
include/llvm/IR/IntrinsicsX86.td | ||
---|---|---|
1348 | Please sync the GCCBuiltin names with gcc. |
lib/Target/X86/X86InstrAVX512.td | ||
---|---|---|
10031 | Go ahead and add BWI here too. It may not be strictly necessary, but a lot of the masking support for bytes and words is dependent on it. It's also consistent with gcc. |
include/llvm/IR/IntrinsicsX86.td | ||
---|---|---|
1348 | basically does weren't yet introduced to gcc, so the closest we can get is Julia Kovel's GFNI respective ongoing patch(s), i.e. https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01043/0002-GF2P8AFFINEINVQB-instruction.patch |
include/llvm/IR/IntrinsicsX86.td | ||
---|---|---|
1348 | does ~~> those |
k
include/llvm/IR/IntrinsicsX86.td | ||
---|---|---|
1348 | They look to have been committed to gcc on Nov. 16. svn revision 254795 |
include/llvm/IR/IntrinsicsX86.td | ||
---|---|---|
1348 | Oh, I see. kyukhin. |
adopted changes proposed by Craig:
maskz variants are out in favor of explicit passing of the additive identity
BWI dependency is explicitly stated
GCC builtins counterparts are matched
+ nice formatting of feature recognition
include/llvm/IR/IntrinsicsX86.td | ||
---|---|---|
1347 | Can we use builtin_ia32_selectb_512 and builtin_ia32_vgf2p8affineinvqb_v16qi to implement these in the clang header instead of adding a separate masked intrinsic? Its different than gcc, but consistent with other intrinsics in clang/llvm. You should add __builtin_ia32_vgf2p8affineinvqb_v64qi without masking so that it can be consistent. | |
1407 | I'd prefer to see this named int_x86_gf2p8affineqb_128 and the 256-bit one named int_x86_gf2p8affineqb_256. | |
1423 | Name this _256 and the one above _128. | |
lib/Target/X86/X86.td | ||
173 | Isn't it Galois not Galios | |
lib/Target/X86/X86ISelLowering.h | ||
582 | Galois | |
lib/Target/X86/X86InstrFragmentsSIMD.td | ||
659 | galois | |
lib/Target/X86/X86InstrSSE.td | ||
8523 | NoVLX should be NoVLX_Or_NoBWI |
include/llvm/IR/IntrinsicsX86.td | ||
---|---|---|
1347 | certainly |
lib/Target/X86/X86.td | ||
---|---|---|
173 | Evariste *Galois*, indeed. |
include/llvm/IR/IntrinsicsX86.td | ||
---|---|---|
1347 | Yep that's what I'm proposing. |
LGTM, but can you also add tests with select instructions for testing that merge masking and zero masking behavior either as part of this or as a follow up.
Can we use builtin_ia32_selectb_512 and builtin_ia32_vgf2p8affineinvqb_v16qi to implement these in the clang header instead of adding a separate masked intrinsic?
Its different than gcc, but consistent with other intrinsics in clang/llvm.
You should add __builtin_ia32_vgf2p8affineinvqb_v64qi without masking so that it can be consistent.