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

[efriedma]

Summary

Otherwise, with D56544, the intrinsic will be expanded to an integer csel, which is probably not what the user expected. This matches the general convention of using "v1" types to represent scalar integer operations in vector registers.

While I'm here, also add some error checking so we don't generate illegal ABS nodes.

[RKSimon]

rebase now that D56544 has landed?

[SjoerdMeijer]

lib/Target/AArch64/AArch64ISelLowering.cpp

2721 ↗

(On Diff #181348)

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.

2726 ↗

(On Diff #181348)

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.

[efriedma]

lib/Target/AArch64/AArch64ISelLowering.cpp

2726 ↗

(On Diff #181348)

Normally I don't like mixing brace styles in a single-if-else (it makes it more confusing to figure out where the chain ends). Not sure how else I would refactor to reduce the number of braces.

[efriedma]

Rebased.

[SjoerdMeijer]

LGTM

llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp

SDValue AArch64TargetLowering::LowerINTRINSIC_WO_CHAIN(SDValue Op,
   unsigned IntNo = cast<ConstantSDNode>(Op.getOperand(0))->getZExtValue();
   SDLoc dl(Op);
   switch (IntNo) {
   default: return SDValue();    // Don't custom lower most intrinsics.
   case Intrinsic::thread_pointer: {
     EVT PtrVT = getPointerTy(DAG.getDataLayout());
     return DAG.getNode(AArch64ISD::THREAD_POINTER, dl, PtrVT);
   }
-  case Intrinsic::aarch64_neon_abs:
-    return DAG.getNode(ISD::ABS, dl, Op.getValueType(),
+  case Intrinsic::aarch64_neon_abs: {
+    EVT Ty = Op.getValueType();
+    if (Ty == MVT::i64) {
+      SDValue Result = DAG.getNode(ISD::BITCAST, dl, MVT::v1i64,
                                    Op.getOperand(1));
+      Result = DAG.getNode(ISD::ABS, dl, MVT::v1i64, Result);
+      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));
+    } else {
+      report_fatal_error("Unexpected type for AArch64 NEON intrinic");
+    }
+  }
   case Intrinsic::aarch64_neon_smax:
     return DAG.getNode(ISD::SMAX, dl, Op.getValueType(),
                        Op.getOperand(1), Op.getOperand(2));
   case Intrinsic::aarch64_neon_umax:
     return DAG.getNode(ISD::UMAX, dl, Op.getValueType(),
                        Op.getOperand(1), Op.getOperand(2));
   case Intrinsic::aarch64_neon_smin:
     return DAG.getNode(ISD::SMIN, dl, Op.getValueType(),

llvm/trunk/test/CodeGen/AArch64/arm64-vabs.ll

 ; CHECK-LABEL: abs_1d:
 ; CHECK: abs d0, d0
   %abs = call <1 x i64> @llvm.aarch64.neon.abs.v1i64(<1 x i64> %A)
   ret <1 x i64> %abs
 }
 
 define i64 @abs_1d_honestly(i64 %A) nounwind {
 ; CHECK-LABEL: abs_1d_honestly:
-; CHECK:       cmp x0, #0
-; CHECK-NEXT:  cneg x0, x0, mi
+; CHECK: abs d0, d0
   %abs = call i64 @llvm.aarch64.neon.abs.i64(i64 %A)
   ret i64 %abs
 }
 
 declare <8 x i8> @llvm.aarch64.neon.abs.v8i8(<8 x i8>) nounwind readnone
 declare <16 x i8> @llvm.aarch64.neon.abs.v16i8(<16 x i8>) nounwind readnone
 declare <4 x i16> @llvm.aarch64.neon.abs.v4i16(<4 x i16>) nounwind readnone
 declare <8 x i16> @llvm.aarch64.neon.abs.v8i16(<8 x i16>) nounwind readnone