DavidSpickett added a comment.
I'd merge https://reviews.llvm.org/D100499 into this given that it's only one
line.
================
Comment at: clang/include/clang/Basic/arm_neon.td:712
+////////////////////////////////////////////////////////////////////////////////
+// Crypto
+// TODO: poly128_t not implemented on aarch32
----------------
This isn't a crypto intrinsic, it just happens to be right after a block like:
```
let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_CRYPTO)" in {
<...>
def SM4EKEY : SInst<"vsm4ekey", "...", "QUi">;
}
```
You could instead say:
```
// Non poly128_t vaddp for Arm and AArch64
```
(keep the todo)
================
Comment at: clang/include/clang/Basic/arm_neon.td:1168
-def VADDP : WInst<"vadd", "...", "PcPsPlQPcQPsQPlQPk">;
+def VADDP : WInst<"vadd", "...", "QPk">;
----------------
Add a comment here, something like `poly128_t vadd for AArch64 only, see
VADDP_Q for the rest`.
Also the def names seem backwards, shouldn't this one be `VADDP_Q` and the one
for Arm and AArch64 be `VADDP`?
Then your comment here is "see VADDP for the rest" which seems correct.
================
Comment at: clang/test/CodeGen/arm-poly-add.c:4
+// RUN: -target-feature +neon \
+// RUN: -target-feature +bf16 -mfloat-abi hard \
+// RUN: -disable-O0-optnone -emit-llvm -o - %s | opt -S -mem2reg \
----------------
You can remove the `+bf16` feature.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100772/new/
https://reviews.llvm.org/D100772
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
