[PATCH] D65931: [AArch64][SVE] Implement abs and neg intrinsics

Tue Sep 3 07:10:45 PDT 2019

greened added inline comments.

================
Comment at: include/llvm/IR/IntrinsicsAArch64.td:756
+let TargetPrefix = "aarch64" in {  // All intrinsics start with "llvm.aarch64.".
+  class AdvSIMD_Merged1VectorArg_Intrinsic
+    : Intrinsic<[llvm_anyvector_ty],
----------------
c-rhodes wrote:
> greened wrote:
> > What does "AdvSIMD" reference?  Why not "SVE?"
> `AdvSIMD` is NEON, some of the existing `AdvSIMD` patterns worked fine for our needs (and some of the new ones weren't exactly SVE specific) so we kept the naming scheme. The SVE specific intrinsics we have downstream are prefixed with `AdvSIMD_SVE`. Happy to change this if it's confusing.
It is a bit jarring to see the "SVE" comment immediately above and then see this definition with "AdvSIMD."  Maybe remove the "SVE" comment since it's not exclusive to SVE?

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65931