[AArch64] Use unsigned variant of <s|u>addv_64 SVE vector reduction intrinsic for 64 bit values

[Rajveer100]

Resolves #157122

When lowering this intrinsic, we are querying the first result type (i.e getValueType(0)) which may not always be true hence giving wrong the extended value type.

As an improvement, for 64 bit values an extend isn't performed regardless of <U/S>ADDV_PRED, so we can directly use the unsigned variant.

[llvmbot]

@llvm/pr-subscribers-backend-aarch64

Author: Rajveer Singh Bharadwaj (Rajveer100)

Changes

Resolves #157122

When lowering this intrinsic, we are querying the first result type (i.e getValueType(0)) which may not always be true hence giving wrong the extended value type.

As an improvement, for 64 bit values an extend isn't performed regardless of <U/S>ADDV_PRED, so we can directly use the unsigned variant.

---

Full diff: https://github.com/llvm/llvm-project/pull/157418.diff

1 Files Affected:

• (modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (+1-5)

diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index aefbbe2534be2..a8d184738c13b 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -22395,11 +22395,7 @@ static SDValue performIntrinsicCombine(SDNode *N,
   case Intrinsic::aarch64_crc32ch:
     return tryCombineCRC32(0xffff, N, DAG);
   case Intrinsic::aarch64_sve_saddv:
-    // There is no i64 version of SADDV because the sign is irrelevant.
-    if (N->getOperand(2)->getValueType(0).getVectorElementType() == MVT::i64)
-      return combineSVEReductionInt(N, AArch64ISD::UADDV_PRED, DAG);
-    else
-      return combineSVEReductionInt(N, AArch64ISD::SADDV_PRED, DAG);
+    return combineSVEReductionInt(N, AArch64ISD::UADDV_PRED, DAG);
   case Intrinsic::aarch64_sve_uaddv:
     return combineSVEReductionInt(N, AArch64ISD::UADDV_PRED, DAG);
   case Intrinsic::aarch64_sve_smaxv:

[davemgreen]

I think it should be something like if (N->getOperand(2).getValueType().getVectorElementType() == MVT::i64)

Can you add a test case from #157122?

[Rajveer100]

Yeah.

[Rajveer100]

Let me know if the added tests work well.

[davemgreen]

Thanks for putting together a fix for this. LGTM