[AArch64] lower abs intrinsic to new ABS instruction in GIsel

[stuij]

Summary

When feature CSSC is available, the abs intrinsic should map to the
new scalar ABS instruction when using GlobalIsel

spec:
https://developer.arm.com/documentation/ddi0602/2022-09/Base-Instructions/ABS--Absolute-value-

[paquette]

llvm/test/CodeGen/AArch64/GlobalISel/select-abs.mir

4 ↗

(On Diff #480445)

The codegen here didn't change at all.

Is it possible to select the scalar abs right now? Should there be a scalar testcase here?

[stuij]

llvm/test/CodeGen/AArch64/GlobalISel/select-abs.mir

4 ↗

(On Diff #480445)

No, there should not be a scalar testcase here. I tested this at some point and must have decided that CHECK-NEXT is better than NEXT, so why not add it to the patch.

[paquette]

llvm/test/CodeGen/AArch64/GlobalISel/select-abs.mir

4 ↗

(On Diff #480445)

Yeah the update_mir_test_checks script was updated a while back to use CHECK-NEXT. Which is probably better, but introduces a lot of noise to the blamelist when we're updating our tests. :(

I'd recommend not updating this test if you're not actually modifying codegen in this patch. If only because it shows up in the blamelist and makes the commit history a little more confusing.

[stuij]

remove changes to select-abs.mir

[aemerson]

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

752–769

Can you do the check outside so we can avoid using predicates?

[stuij]

address review comment

[aemerson]

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

757–760

Am I reading this wrong or can't this be:

.legalFor({s32, s64})
.legalIf(typeInSet(0, LegalABSVectorTypes)

[stuij]

addressed review comment

[stuij]

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

757–760

Ah right :) Thanks! I didn't consider multiple predicates for the same purpose could work. Even though almost this exact scenario is used elsewhere in the code, I see now. I should put time into understanding the engine a bit better.

[aemerson]

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

758–760

Sorry, just spotted something else. This rule now is partially redundant since {s32,s64} are legal. It's just .lowerIf(isScalar(0)), which also means that most of this can be merged with the non-CSSC rules. You can do:

if (HasCSSC)
  ABSActions
    .legalFor({s32, s64})
ABSActions
  .legalFor(PackedVectorAllTypeList)
  .lowerIf(isScalar(0));

[stuij]

simplified code some more

[stuij]

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

757–760

thanks. done!

[aemerson]

LGTM, thanks!

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

 //===- AArch64LegalizerInfo.cpp ----------------------------------*- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 /// \file
if (ST.hasMOPS()) {
     getActionDefinitionsBuilder(G_MEMCPY_INLINE)
         .legalForCartesianProduct({p0}, {p0}, {s64});
 
   } else {
     getActionDefinitionsBuilder({G_BZERO, G_MEMCPY, G_MEMMOVE, G_MEMSET})
         .libcall();
   }
 
-  // FIXME: Legal types are only legal with NEON.
-  getActionDefinitionsBuilder(G_ABS)
-      .lowerIf(isScalar(0))
-      .legalFor(PackedVectorAllTypeList);
+  SmallVector<LLT, 4> LegalABSVectorTypes = PackedVectorAllTypeList;
+  // FIXME: Legal vector types are only legal with NEON.
+  auto &ABSActions = getActionDefinitionsBuilder(G_ABS);
+  if (HasCSSC)
+    ABSActions
+        .legalIf([=](const LegalityQuery &Query) {
+          return typeInSet(0, {s32, s64})(Query) ||
+              llvm::is_contained(LegalABSVectorTypes, Query.Types[0]);
+        })
+        .lowerIf([=](const LegalityQuery &Query) {
+          return isScalar(0)(Query) &&
+              !typeInSet(0, {s32, s64})(Query);
+        });
+  else
+    ABSActions
+        .legalFor(PackedVectorAllTypeList)
+        .lowerIf(isScalar(0));
 
   getActionDefinitionsBuilder(G_VECREDUCE_FADD)
       // We only have FADDP to do reduction-like operations. Lower the rest.
       .legalFor({{s32, v2s32}, {s64, v2s64}})
       .clampMaxNumElements(1, s64, 2)
       .clampMaxNumElements(1, s32, 2)
       .lower();
 
   getActionDefinitionsBuilder(G_VECREDUCE_ADD)

llvm/test/CodeGen/AArch64/GlobalISel/legalize-abs.mir

 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
 # RUN: llc -mtriple=aarch64 -run-pass=legalizer -global-isel-abort=1 %s -o - | FileCheck %s
+# RUN: llc -mtriple=aarch64 -run-pass=legalizer -mattr=+cssc -global-isel-abort=1 %s -o - | FileCheck %s --check-prefix=CHECK-CSSC
 ---
 name:            abs_s32
 liveins:
 body:             |
   bb.0:
     ; CHECK-LABEL: name: abs_s32
     ; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
-    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 31
-    ; CHECK: [[ASHR:%[0-9]+]]:_(s32) = G_ASHR [[COPY]], [[C]](s64)
-    ; CHECK: [[ADD:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[ASHR]]
-    ; CHECK: [[XOR:%[0-9]+]]:_(s32) = G_XOR [[ADD]], [[ASHR]]
-    ; CHECK: $w0 = COPY [[XOR]](s32)
+    ; CHECK-NEXT: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 31
+    ; CHECK-NEXT: [[ASHR:%[0-9]+]]:_(s32) = G_ASHR [[COPY]], [[C]](s64)
+    ; CHECK-NEXT: [[ADD:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[ASHR]]
+    ; CHECK-NEXT: [[XOR:%[0-9]+]]:_(s32) = G_XOR [[ADD]], [[ASHR]]
+    ; CHECK-NEXT: $w0 = COPY [[XOR]](s32)
+    ; CHECK-CSSC-LABEL: name: abs_s32
+    ; CHECK-CSSC: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+    ; CHECK-CSSC-NEXT: [[ABS:%[0-9]+]]:_(s32) = G_ABS [[COPY]]
+    ; CHECK-CSSC-NEXT: $w0 = COPY [[ABS]](s32)
     %0:_(s32) = COPY $w0
     %1:_(s32) = G_ABS %0(s32)
     $w0 = COPY %1(s32)
 ...
 ---
 name:            abs_s64
 liveins:
 body:             |
   bb.0:
     ; CHECK-LABEL: name: abs_s64
     ; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $x0
-    ; CHECK: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 63
-    ; CHECK: [[ASHR:%[0-9]+]]:_(s64) = G_ASHR [[COPY]], [[C]](s64)
-    ; CHECK: [[ADD:%[0-9]+]]:_(s64) = G_ADD [[COPY]], [[ASHR]]
-    ; CHECK: [[XOR:%[0-9]+]]:_(s64) = G_XOR [[ADD]], [[ASHR]]
-    ; CHECK: $x0 = COPY [[XOR]](s64)
+    ; CHECK-NEXT: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 63
+    ; CHECK-NEXT: [[ASHR:%[0-9]+]]:_(s64) = G_ASHR [[COPY]], [[C]](s64)
+    ; CHECK-NEXT: [[ADD:%[0-9]+]]:_(s64) = G_ADD [[COPY]], [[ASHR]]
+    ; CHECK-NEXT: [[XOR:%[0-9]+]]:_(s64) = G_XOR [[ADD]], [[ASHR]]
+    ; CHECK-NEXT: $x0 = COPY [[XOR]](s64)
+    ; CHECK-CSSC-LABEL: name: abs_s64
+    ; CHECK-CSSC: [[COPY:%[0-9]+]]:_(s64) = COPY $x0
+    ; CHECK-CSSC-NEXT: [[ABS:%[0-9]+]]:_(s64) = G_ABS [[COPY]]
+    ; CHECK-CSSC-NEXT: $x0 = COPY [[ABS]](s64)
     %0:_(s64) = COPY $x0
     %1:_(s64) = G_ABS %0(s64)
     $x0 = COPY %1(s64)
 ...
 ---
 name:            abs_v4s16
 tracksRegLiveness: true
 body:             |
   bb.0:
     liveins: $d0
 
     ; CHECK-LABEL: name: abs_v4s16
     ; CHECK: liveins: $d0
-    ; CHECK: [[COPY:%[0-9]+]]:_(<4 x s16>) = COPY $d0
-    ; CHECK: [[ABS:%[0-9]+]]:_(<4 x s16>) = G_ABS [[COPY]]
-    ; CHECK: $d0 = COPY [[ABS]](<4 x s16>)
-    ; CHECK: RET_ReallyLR implicit $d0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<4 x s16>) = COPY $d0
+    ; CHECK-NEXT: [[ABS:%[0-9]+]]:_(<4 x s16>) = G_ABS [[COPY]]
+    ; CHECK-NEXT: $d0 = COPY [[ABS]](<4 x s16>)
+    ; CHECK-NEXT: RET_ReallyLR implicit $d0
+    ; CHECK-CSSC-LABEL: name: abs_v4s16
+    ; CHECK-CSSC: liveins: $d0
+    ; CHECK-CSSC-NEXT: {{  $}}
+    ; CHECK-CSSC-NEXT: [[COPY:%[0-9]+]]:_(<4 x s16>) = COPY $d0
+    ; CHECK-CSSC-NEXT: [[ABS:%[0-9]+]]:_(<4 x s16>) = G_ABS [[COPY]]
+    ; CHECK-CSSC-NEXT: $d0 = COPY [[ABS]](<4 x s16>)
+    ; CHECK-CSSC-NEXT: RET_ReallyLR implicit $d0
     %0:_(<4 x s16>) = COPY $d0
     %1:_(<4 x s16>) = G_ABS %0
     $d0 = COPY %1(<4 x s16>)
     RET_ReallyLR implicit $d0
 
 ...
 ---
 name:            abs_v8s16
 tracksRegLiveness: true
 body:             |
   bb.0:
     liveins: $q0
 
     ; CHECK-LABEL: name: abs_v8s16
     ; CHECK: liveins: $q0
-    ; CHECK: [[COPY:%[0-9]+]]:_(<8 x s16>) = COPY $q0
-    ; CHECK: [[ABS:%[0-9]+]]:_(<8 x s16>) = G_ABS [[COPY]]
-    ; CHECK: $q0 = COPY [[ABS]](<8 x s16>)
-    ; CHECK: RET_ReallyLR implicit $q0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<8 x s16>) = COPY $q0
+    ; CHECK-NEXT: [[ABS:%[0-9]+]]:_(<8 x s16>) = G_ABS [[COPY]]
+    ; CHECK-NEXT: $q0 = COPY [[ABS]](<8 x s16>)
+    ; CHECK-NEXT: RET_ReallyLR implicit $q0
+    ; CHECK-CSSC-LABEL: name: abs_v8s16
+    ; CHECK-CSSC: liveins: $q0
+    ; CHECK-CSSC-NEXT: {{  $}}
+    ; CHECK-CSSC-NEXT: [[COPY:%[0-9]+]]:_(<8 x s16>) = COPY $q0
+    ; CHECK-CSSC-NEXT: [[ABS:%[0-9]+]]:_(<8 x s16>) = G_ABS [[COPY]]
+    ; CHECK-CSSC-NEXT: $q0 = COPY [[ABS]](<8 x s16>)
+    ; CHECK-CSSC-NEXT: RET_ReallyLR implicit $q0
     %0:_(<8 x s16>) = COPY $q0
     %1:_(<8 x s16>) = G_ABS %0
     $q0 = COPY %1(<8 x s16>)
     RET_ReallyLR implicit $q0
 
 ...
 ---
 name:            abs_v2s32
 tracksRegLiveness: true
 body:             |
   bb.0:
     liveins: $d0
 
     ; CHECK-LABEL: name: abs_v2s32
     ; CHECK: liveins: $d0
-    ; CHECK: [[COPY:%[0-9]+]]:_(<2 x s32>) = COPY $d0
-    ; CHECK: [[ABS:%[0-9]+]]:_(<2 x s32>) = G_ABS [[COPY]]
-    ; CHECK: $d0 = COPY [[ABS]](<2 x s32>)
-    ; CHECK: RET_ReallyLR implicit $d0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<2 x s32>) = COPY $d0
+    ; CHECK-NEXT: [[ABS:%[0-9]+]]:_(<2 x s32>) = G_ABS [[COPY]]
+    ; CHECK-NEXT: $d0 = COPY [[ABS]](<2 x s32>)
+    ; CHECK-NEXT: RET_ReallyLR implicit $d0
+    ; CHECK-CSSC-LABEL: name: abs_v2s32
+    ; CHECK-CSSC: liveins: $d0
+    ; CHECK-CSSC-NEXT: {{  $}}
+    ; CHECK-CSSC-NEXT: [[COPY:%[0-9]+]]:_(<2 x s32>) = COPY $d0
+    ; CHECK-CSSC-NEXT: [[ABS:%[0-9]+]]:_(<2 x s32>) = G_ABS [[COPY]]
+    ; CHECK-CSSC-NEXT: $d0 = COPY [[ABS]](<2 x s32>)
+    ; CHECK-CSSC-NEXT: RET_ReallyLR implicit $d0
     %0:_(<2 x s32>) = COPY $d0
     %1:_(<2 x s32>) = G_ABS %0
     $d0 = COPY %1(<2 x s32>)
     RET_ReallyLR implicit $d0
 
 ...
 ---
 name:            abs_v4s32
 tracksRegLiveness: true
 body:             |
   bb.0:
     liveins: $q0
 
     ; CHECK-LABEL: name: abs_v4s32
     ; CHECK: liveins: $q0
-    ; CHECK: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
-    ; CHECK: [[ABS:%[0-9]+]]:_(<4 x s32>) = G_ABS [[COPY]]
-    ; CHECK: $q0 = COPY [[ABS]](<4 x s32>)
-    ; CHECK: RET_ReallyLR implicit $q0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
+    ; CHECK-NEXT: [[ABS:%[0-9]+]]:_(<4 x s32>) = G_ABS [[COPY]]
+    ; CHECK-NEXT: $q0 = COPY [[ABS]](<4 x s32>)
+    ; CHECK-NEXT: RET_ReallyLR implicit $q0
+    ; CHECK-CSSC-LABEL: name: abs_v4s32
+    ; CHECK-CSSC: liveins: $q0
+    ; CHECK-CSSC-NEXT: {{  $}}
+    ; CHECK-CSSC-NEXT: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
+    ; CHECK-CSSC-NEXT: [[ABS:%[0-9]+]]:_(<4 x s32>) = G_ABS [[COPY]]
+    ; CHECK-CSSC-NEXT: $q0 = COPY [[ABS]](<4 x s32>)
+    ; CHECK-CSSC-NEXT: RET_ReallyLR implicit $q0
     %0:_(<4 x s32>) = COPY $q0
     %1:_(<4 x s32>) = G_ABS %0
     $q0 = COPY %1(<4 x s32>)
     RET_ReallyLR implicit $q0
 
 ...
 ---
 name:            abs_v4s8
 tracksRegLiveness: true
 body:             |
   bb.0:
     liveins: $d0
 
     ; CHECK-LABEL: name: abs_v4s8
     ; CHECK: liveins: $d0
-    ; CHECK: [[COPY:%[0-9]+]]:_(<8 x s8>) = COPY $d0
-    ; CHECK: [[ABS:%[0-9]+]]:_(<8 x s8>) = G_ABS [[COPY]]
-    ; CHECK: $d0 = COPY [[ABS]](<8 x s8>)
-    ; CHECK: RET_ReallyLR implicit $d0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<8 x s8>) = COPY $d0
+    ; CHECK-NEXT: [[ABS:%[0-9]+]]:_(<8 x s8>) = G_ABS [[COPY]]
+    ; CHECK-NEXT: $d0 = COPY [[ABS]](<8 x s8>)
+    ; CHECK-NEXT: RET_ReallyLR implicit $d0
+    ; CHECK-CSSC-LABEL: name: abs_v4s8
+    ; CHECK-CSSC: liveins: $d0
+    ; CHECK-CSSC-NEXT: {{  $}}
+    ; CHECK-CSSC-NEXT: [[COPY:%[0-9]+]]:_(<8 x s8>) = COPY $d0
+    ; CHECK-CSSC-NEXT: [[ABS:%[0-9]+]]:_(<8 x s8>) = G_ABS [[COPY]]
+    ; CHECK-CSSC-NEXT: $d0 = COPY [[ABS]](<8 x s8>)
+    ; CHECK-CSSC-NEXT: RET_ReallyLR implicit $d0
     %0:_(<8 x s8>) = COPY $d0
     %1:_(<8 x s8>) = G_ABS %0
     $d0 = COPY %1(<8 x s8>)
     RET_ReallyLR implicit $d0
 
 ...
 ---
 name:            abs_v16s8
 tracksRegLiveness: true
 body:             |
   bb.0:
     liveins: $q0
 
     ; CHECK-LABEL: name: abs_v16s8
     ; CHECK: liveins: $q0
-    ; CHECK: [[COPY:%[0-9]+]]:_(<16 x s8>) = COPY $q0
-    ; CHECK: [[ABS:%[0-9]+]]:_(<16 x s8>) = G_ABS [[COPY]]
-    ; CHECK: $q0 = COPY [[ABS]](<16 x s8>)
-    ; CHECK: RET_ReallyLR implicit $q0
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<16 x s8>) = COPY $q0
+    ; CHECK-NEXT: [[ABS:%[0-9]+]]:_(<16 x s8>) = G_ABS [[COPY]]
+    ; CHECK-NEXT: $q0 = COPY [[ABS]](<16 x s8>)
+    ; CHECK-NEXT: RET_ReallyLR implicit $q0
+    ; CHECK-CSSC-LABEL: name: abs_v16s8
+    ; CHECK-CSSC: liveins: $q0
+    ; CHECK-CSSC-NEXT: {{  $}}
+    ; CHECK-CSSC-NEXT: [[COPY:%[0-9]+]]:_(<16 x s8>) = COPY $q0
+    ; CHECK-CSSC-NEXT: [[ABS:%[0-9]+]]:_(<16 x s8>) = G_ABS [[COPY]]
+    ; CHECK-CSSC-NEXT: $q0 = COPY [[ABS]](<16 x s8>)
+    ; CHECK-CSSC-NEXT: RET_ReallyLR implicit $q0
     %0:_(<16 x s8>) = COPY $q0
     %1:_(<16 x s8>) = G_ABS %0
     $q0 = COPY %1(<16 x s8>)
     RET_ReallyLR implicit $q0
 
 ...