[PATCH] D56616: [AArch64] Explicitly use v1i64 type for llvm.aarch64.neon.abs.i64 .

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:2721
+    EVT Ty = Op.getValueType();
+    if (Ty == MVT::i64) {
+      SDValue Result = DAG.getNode(ISD::BITCAST, dl, MVT::v1i64,
----------------
If I understand things correctly,  D56544 is committed, so that means this intrinsic is lowered to different ISD nodes but codegen is the same. And a quick grep suggests this is already covered by test arm64-vabs.ll.




================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:2726
+      return DAG.getNode(ISD::BITCAST, dl, MVT::i64, Result);
+    } else if (Ty.isVector() && Ty.isInteger() && isTypeLegal(Ty)) {
+      return DAG.getNode(ISD::ABS, dl, Ty, Op.getOperand(1));
----------------
And this is covered by arm64-vabs.ll too. I.e. the tests with llvm.aarch64.neon.abs.v4i32 etc.

In that case my only nitpick is that we don't need all the curly brackets here but it LGTM to me otherwise.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56616