[AArch64][GISel] Support neon.abs intrinsic for vector types

[madhur13490]

This patch adds support in instruction selection
aarch64.neon.abs intrinsic for vector types.

[llvmbot]

@llvm/pr-subscribers-backend-aarch64

Author: Madhur Amilkanthwar (madhur13490)

Changes

This patch add support in instruction selection
aarch64.neon.abs intrinsic for vector types.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp (+28)
• (modified) llvm/test/CodeGen/AArch64/arm64-vabs.ll (+1-7)

diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index 18361cf3685642..69f7d07403210c 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -6535,6 +6535,34 @@ bool AArch64InstructionSelector::selectIntrinsic(MachineInstr &I,
   switch (IntrinID) {
   default:
     break;
+  case Intrinsic::aarch64_neon_abs: {
+    Register DstReg = I.getOperand(0).getReg();
+    Register SrcReg = I.getOperand(2).getReg();
+    auto SrcRegType = MRI.getType(SrcReg);
+    unsigned Opc = 0;
+
+    if (SrcRegType == LLT::fixed_vector(8, 8)) {
+      Opc = AArch64::ABSv8i8;
+    } else if (SrcRegType == LLT::fixed_vector(16, 8)) {
+      Opc = AArch64::ABSv16i8;
+    } else if (SrcRegType == LLT::fixed_vector(4, 16)) {
+      Opc = AArch64::ABSv4i16;
+    } else if (SrcRegType == LLT::fixed_vector(8, 16)) {
+      Opc = AArch64::ABSv8i16;
+    } else if (SrcRegType == LLT::fixed_vector(2, 32)) {
+      Opc = AArch64::ABSv2i32;
+    } else if (SrcRegType == LLT::fixed_vector(4, 32)) {
+      Opc = AArch64::ABSv4i32;
+    } else {
+      LLVM_DEBUG(dbgs() << "Unhandled type for aarch64.neon.abs intrinsic");
+      return false;
+    }
+    auto ABSInst = MIB.buildInstr(Opc, {DstReg}, {SrcReg});
+    constrainSelectedInstRegOperands(*ABSInst, TII, TRI, RBI);
+
+    I.eraseFromParent();
+    return true;
+  }
   case Intrinsic::aarch64_crypto_sha1h: {
     Register DstReg = I.getOperand(0).getReg();
     Register SrcReg = I.getOperand(2).getReg();
diff --git a/llvm/test/CodeGen/AArch64/arm64-vabs.ll b/llvm/test/CodeGen/AArch64/arm64-vabs.ll
index 48afcc5c3dd2b6..56436930afc14e 100644
--- a/llvm/test/CodeGen/AArch64/arm64-vabs.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-vabs.ll
@@ -2,13 +2,7 @@
 ; RUN: llc < %s -mtriple=arm64-eabi -aarch64-neon-syntax=apple | FileCheck -check-prefixes=CHECK,CHECK-SD %s
 ; RUN: llc < %s -mtriple=arm64-eabi -aarch64-neon-syntax=apple -global-isel -global-isel-abort=2 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-GI
 
-; CHECK-GI:       warning: Instruction selection used fallback path for abs_8b
-; CHECK-GI-NEXT:  warning: Instruction selection used fallback path for abs_16b
-; CHECK-GI-NEXT:  warning: Instruction selection used fallback path for abs_4h
-; CHECK-GI-NEXT:  warning: Instruction selection used fallback path for abs_8h
-; CHECK-GI-NEXT:  warning: Instruction selection used fallback path for abs_2s
-; CHECK-GI-NEXT:  warning: Instruction selection used fallback path for abs_4s
-; CHECK-GI-NEXT:  warning: Instruction selection used fallback path for abs_1d
+; CHECK-GI:  warning: Instruction selection used fallback path for abs_1d
 ; CHECK-GI-NEXT:  warning: Instruction selection used fallback path for abs_1d_honestly
 ; CHECK-GI-NEXT:  warning: Instruction selection used fallback path for fabds
 ; CHECK-GI-NEXT:  warning: Instruction selection used fallback path for fabdd

[davemgreen]

If we don't have tablegen patterns for these intrinsics it is probably best to do the same as ISel and convert them into a G_ABS, which should be selected naturally.

(Considering the intrinsic, which as far as I understand always works like an abs instruction, it would be nice if aarch64_neon_abs was not generated at-all and it used llvm.abs everywhere instead, but that is a much larger change that is outside the scope of getting them working with gisel).

I believe there are other intrinsics that should work the same way. Perhaps a combine could be added to replace the intrinsic with abs, early enough that the other optimizations we would ideally want can happen. The good thing about doing it earlier is it shouldn't have to be as careful about all the different types, it can just convert the intrinsic.

[davemgreen]

Oh - AArch64LegalizerInfo::legalizeIntrinsic() has some existing lowering for intrinsics like min/max. That is probably a good place for it.

[arsenm]

This looks like it should be easily handled with patterns

[madhur13490]

@arsenm @davemgreen
Please have a look. I made the changes as per @davemgreen's suggestion.

[davemgreen]

Thanks, LGTM