Skip to content

[GlobalIsel] Canonicalize G_ICMP #108755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Sep 16, 2024
Merged

[GlobalIsel] Canonicalize G_ICMP #108755

merged 3 commits into from
Sep 16, 2024

Conversation

tschuett
Copy link

@tschuett tschuett commented Sep 15, 2024 •
edited
Loading

As a side-effect, we start constant folding icmps.

Split out from #105991.

@tschuett
[GlobalIsel] Canonicalize G_ICMP
3730790 
As a side-effect, we start constant folding icmps.
@tschuett tschuett requested a review from aemerson September 15, 2024 13:37
@llvmbot
Copy link
Member

llvmbot commented Sep 15, 2024 •
edited
Loading

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-backend-aarch64

Author: Thorsten Schütt (tschuett)

Changes

As a side-effect, we start constant folding icmps.

Patch is 35.25 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/108755.diff

11 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h (+6)
  • (modified) llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h (+10)
  • (modified) llvm/include/llvm/CodeGen/GlobalISel/Utils.h (+22)
  • (modified) llvm/include/llvm/Target/GlobalISel/Combine.td (+18-7)
  • (modified) llvm/lib/CodeGen/GlobalISel/CMakeLists.txt (+1)
  • (added) llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp (+86)
  • (modified) llvm/lib/CodeGen/GlobalISel/Utils.cpp (+40)
  • (added) llvm/test/CodeGen/AArch64/GlobalISel/combine-visit-icmp.mir (+95)
  • (modified) llvm/test/CodeGen/AArch64/arm64-ccmp.ll (+18-42)
  • (modified) llvm/test/CodeGen/AMDGPU/GlobalISel/fdiv.f64.ll (+9-9)
  • (modified) llvm/test/CodeGen/AMDGPU/rsq.f64.ll (+23-23)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
index 828532dcffb7d3..37c9422d192754 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
+#include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/Register.h"
 #include "llvm/CodeGenTypes/LowLevelType.h"
 #include "llvm/IR/InstrTypes.h"
@@ -909,6 +910,8 @@ class CombinerHelper {
   bool matchCastOfBuildVector(const MachineInstr &CastMI,
                               const MachineInstr &BVMI, BuildFnTy &MatchInfo);
 
+  bool matchCanonicalizeICmp(const MachineInstr &MI, BuildFnTy &MatchInfo);
+
 private:
   /// Checks for legality of an indexed variant of \p LdSt.
   bool isIndexedLoadStoreLegal(GLoadStore &LdSt) const;
@@ -1023,6 +1026,9 @@ class CombinerHelper {
   bool tryFoldLogicOfFCmps(GLogicalBinOp *Logic, BuildFnTy &MatchInfo);
 
   bool isCastFree(unsigned Opcode, LLT ToTy, LLT FromTy) const;
+
+  bool constantFoldICmp(const GICmp &ICmp, const GIConstant &LHSCst,
+                        const GIConstant &RHSCst, BuildFnTy &MatchInfo);
 };
 } // namespace llvm
 
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h b/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
index ef1171d9f1f64d..2c459ccdd8a731 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
@@ -950,6 +950,16 @@ class GExtOrTruncOp : public GCastOp {
   };
 };
 
+/// Represents a splat vector.
+class GSplatVector : public GenericMachineInstr {
+public:
+  Register getValueReg() const { return getOperand(1).getReg(); }
+
+  static bool classof(const MachineInstr *MI) {
+    return MI->getOpcode() == TargetOpcode::G_SPLAT_VECTOR;
+  };
+};
+
 } // namespace llvm
 
 #endif // LLVM_CODEGEN_GLOBALISEL_GENERICMACHINEINSTRS_H
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
index cf5fd6d6f288bd..4aeacfdf4ecdf0 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -593,5 +593,27 @@ bool isGuaranteedNotToBeUndef(Register Reg, const MachineRegisterInfo &MRI,
 /// estimate of the type.
 Type *getTypeForLLT(LLT Ty, LLVMContext &C);
 
+enum class GIConstantKind { Scalar, FixedVector, ScalableVector };
+
+/// An integer-like constant.
+class GIConstant {
+  GIConstantKind Kind;
+  SmallVector<APInt> Values;
+  APInt Value;
+
+public:
+  GIConstant(ArrayRef<APInt> Values)
+      : Kind(GIConstantKind::FixedVector), Values(Values) {};
+  GIConstant(const APInt &Value, GIConstantKind Kind)
+      : Kind(Kind), Value(Value) {};
+
+  GIConstantKind getKind() const { return Kind; }
+
+  APInt getScalarValue() const;
+
+  static std::optional<GIConstant> getConstant(Register Const,
+                                               const MachineRegisterInfo &MRI);
+};
+
 } // End namespace llvm.
 #endif
diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td
index a595a51d7b01ff..c66212d2ab12c8 100644
--- a/llvm/include/llvm/Target/GlobalISel/Combine.td
+++ b/llvm/include/llvm/Target/GlobalISel/Combine.td
@@ -1007,9 +1007,6 @@ def double_icmp_zero_or_combine: GICombineRule<
          (G_ICMP $root, $p, $ordst, 0))
 >;
 
-def double_icmp_zero_and_or_combine : GICombineGroup<[double_icmp_zero_and_combine,
-                                                      double_icmp_zero_or_combine]>;
-
 def and_or_disjoint_mask : GICombineRule<
   (defs root:$root, build_fn_matchinfo:$info),
   (match (wip_match_opcode G_AND):$root,
@@ -1918,6 +1915,20 @@ def cast_combines: GICombineGroup<[
   integer_of_truncate
 ]>;
 
+def canonicalize_icmp : GICombineRule<
+  (defs root:$root, build_fn_matchinfo:$matchinfo),
+  (match (G_ICMP $root, $pred, $lhs, $rhs):$cmp,
+         [{ return Helper.matchCanonicalizeICmp(*${cmp}, ${matchinfo}); }]),
+  (apply [{ Helper.applyBuildFn(*${cmp}, ${matchinfo}); }])>;
+
+def icmp_combines: GICombineGroup<[
+  canonicalize_icmp,
+  icmp_to_true_false_known_bits,
+  icmp_to_lhs_known_bits,
+  double_icmp_zero_and_combine,
+  double_icmp_zero_or_combine,
+  redundant_binop_in_equality
+]>;
 
 // FIXME: These should use the custom predicate feature once it lands.
 def undef_combines : GICombineGroup<[undef_to_fp_zero, undef_to_int_zero,
@@ -1951,7 +1962,7 @@ def const_combines : GICombineGroup<[constant_fold_fp_ops, const_ptradd_to_i2p,
 
 def known_bits_simplifications : GICombineGroup<[
   redundant_and, redundant_sext_inreg, redundant_or, urem_pow2_to_mask,
-  zext_trunc_fold, icmp_to_true_false_known_bits, icmp_to_lhs_known_bits,
+  zext_trunc_fold,
   sext_inreg_to_zext_inreg]>;
 
 def width_reduction_combines : GICombineGroup<[reduce_shl_of_extend,
@@ -1984,7 +1995,7 @@ def all_combines : GICombineGroup<[integer_reassoc_combines, trivial_combines,
     combine_extracted_vector_load,
     undef_combines, identity_combines, phi_combines,
     simplify_add_to_sub, hoist_logic_op_with_same_opcode_hands, shifts_too_big,
-    reassocs, ptr_add_immed_chain,
+    reassocs, ptr_add_immed_chain, icmp_combines,
     shl_ashr_to_sext_inreg, sext_inreg_of_load,
     width_reduction_combines, select_combines,
     known_bits_simplifications,
@@ -1998,9 +2009,9 @@ def all_combines : GICombineGroup<[integer_reassoc_combines, trivial_combines,
     constant_fold_cast_op, fabs_fneg_fold,
     intdiv_combines, mulh_combines, redundant_neg_operands,
     and_or_disjoint_mask, fma_combines, fold_binop_into_select,
-    sub_add_reg, select_to_minmax, redundant_binop_in_equality,
+    sub_add_reg, select_to_minmax, 
     fsub_to_fneg, commute_constant_to_rhs, match_ands, match_ors,
-    combine_concat_vector, double_icmp_zero_and_or_combine, match_addos,
+    combine_concat_vector, match_addos,
     sext_trunc, zext_trunc, prefer_sign_combines, combine_shuffle_concat]>;
 
 // A combine group used to for prelegalizer combiners at -O0. The combines in
diff --git a/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt b/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
index a15b76440364b1..af1717dbf76f39 100644
--- a/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
+++ b/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
@@ -7,6 +7,7 @@ add_llvm_component_library(LLVMGlobalISel
   Combiner.cpp
   CombinerHelper.cpp
   CombinerHelperCasts.cpp
+  CombinerHelperCompares.cpp
   CombinerHelperVectorOps.cpp
   GIMatchTableExecutor.cpp
   GISelChangeObserver.cpp
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp
new file mode 100644
index 00000000000000..39ac4ee205fb7a
--- /dev/null
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp
@@ -0,0 +1,86 @@
+//===- CombinerHelperCompares.cpp------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements CombinerHelper for G_ICMP.
+//
+//===----------------------------------------------------------------------===//
+#include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
+#include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
+#include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
+#include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
+#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
+#include "llvm/CodeGen/GlobalISel/Utils.h"
+#include "llvm/CodeGen/LowLevelTypeUtils.h"
+#include "llvm/CodeGen/MachineInstr.h"
+#include "llvm/CodeGen/MachineOperand.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/TargetOpcodes.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
+#include <cstdlib>
+
+#define DEBUG_TYPE "gi-combiner"
+
+using namespace llvm;
+
+bool CombinerHelper::constantFoldICmp(const GICmp &ICmp,
+                                      const GIConstant &LHSCst,
+                                      const GIConstant &RHSCst,
+                                      BuildFnTy &MatchInfo) {
+  if (LHSCst.getKind() != GIConstantKind::Scalar)
+    return false;
+
+  Register Dst = ICmp.getReg(0);
+  LLT DstTy = MRI.getType(Dst);
+
+  if (!isConstantLegalOrBeforeLegalizer(DstTy))
+    return false;
+
+  CmpInst::Predicate Pred = ICmp.getCond();
+  APInt LHS = LHSCst.getScalarValue();
+  APInt RHS = RHSCst.getScalarValue();
+
+  bool Result = ICmpInst::compare(LHS, RHS, Pred);
+
+  MatchInfo = [=](MachineIRBuilder &B) {
+    if (Result)
+      B.buildConstant(Dst, getICmpTrueVal(getTargetLowering(),
+                                          /*IsVector=*/DstTy.isVector(),
+                                          /*IsFP=*/false));
+    else
+      B.buildConstant(Dst, 0);
+  };
+
+  return true;
+}
+
+bool CombinerHelper::matchCanonicalizeICmp(const MachineInstr &MI,
+                                           BuildFnTy &MatchInfo) {
+  const GICmp *Cmp = cast<GICmp>(&MI);
+
+  Register Dst = Cmp->getReg(0);
+  Register LHS = Cmp->getLHSReg();
+  Register RHS = Cmp->getRHSReg();
+
+  CmpInst::Predicate Pred = Cmp->getCond();
+  assert(CmpInst::isIntPredicate(Pred) && "Not an integer compare!");
+  if (auto CLHS = GIConstant::getConstant(LHS, MRI)) {
+    if (auto CRHS = GIConstant::getConstant(RHS, MRI))
+      return constantFoldICmp(*Cmp, *CLHS, *CRHS, MatchInfo);
+
+    // If we have a constant, make sure it is on the RHS.
+    std::swap(LHS, RHS);
+    Pred = CmpInst::getSwappedPredicate(Pred);
+
+    MatchInfo = [=](MachineIRBuilder &B) { B.buildICmp(Pred, Dst, LHS, RHS); };
+    return true;
+  }
+
+  return false;
+}
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 1713a582d5cfe5..396f0f07ae9050 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -1968,3 +1968,43 @@ Type *llvm::getTypeForLLT(LLT Ty, LLVMContext &C) {
                            Ty.getElementCount());
   return IntegerType::get(C, Ty.getSizeInBits());
 }
+
+APInt llvm::GIConstant::getScalarValue() const {
+  assert(Kind == GIConstantKind::Scalar && "Expected scalar constant");
+
+  return Value;
+}
+
+std::optional<GIConstant>
+llvm::GIConstant::getConstant(Register Const, const MachineRegisterInfo &MRI) {
+  MachineInstr *Constant = getDefIgnoringCopies(Const, MRI);
+
+  if (GSplatVector *Splat = dyn_cast<GSplatVector>(Constant)) {
+    std::optional<ValueAndVReg> MayBeConstant =
+        getIConstantVRegValWithLookThrough(Splat->getValueReg(), MRI);
+    if (!MayBeConstant)
+      return std::nullopt;
+    return GIConstant(MayBeConstant->Value, GIConstantKind::ScalableVector);
+  }
+
+  if (GBuildVector *Build = dyn_cast<GBuildVector>(Constant)) {
+    SmallVector<APInt> Values;
+    unsigned NumSources = Build->getNumSources();
+    for (unsigned I = 0; I < NumSources; ++I) {
+      Register SrcReg = Build->getSourceReg(I);
+      std::optional<ValueAndVReg> MayBeConstant =
+          getIConstantVRegValWithLookThrough(SrcReg, MRI);
+      if (!MayBeConstant)
+        return std::nullopt;
+      Values.push_back(MayBeConstant->Value);
+    }
+    return GIConstant(Values);
+  }
+
+  std::optional<ValueAndVReg> MayBeConstant =
+      getIConstantVRegValWithLookThrough(Const, MRI);
+  if (!MayBeConstant)
+    return std::nullopt;
+
+  return GIConstant(MayBeConstant->Value, GIConstantKind::Scalar);
+}
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/combine-visit-icmp.mir b/llvm/test/CodeGen/AArch64/GlobalISel/combine-visit-icmp.mir
new file mode 100644
index 00000000000000..bf04ac02d086ab
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/combine-visit-icmp.mir
@@ -0,0 +1,95 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -o - -mtriple=aarch64-unknown-unknown -run-pass=aarch64-prelegalizer-combiner -verify-machineinstrs  %s | FileCheck %s --check-prefixes=CHECK
+
+---
+name:            test_icmp_canon
+body:             |
+  bb.1:
+    ; CHECK-LABEL: name: test_icmp_canon
+    ; CHECK: %lhs:_(s64) = G_CONSTANT i64 11
+    ; CHECK-NEXT: %rhs:_(s64) = COPY $x0
+    ; CHECK-NEXT: %res:_(s32) = G_ICMP intpred(sgt), %rhs(s64), %lhs
+    ; CHECK-NEXT: $w0 = COPY %res(s32)
+    %lhs:_(s64) = G_CONSTANT i64 11
+    %rhs:_(s64) = COPY $x0
+    %res:_(s32) = G_ICMP intpred(slt), %lhs(s64), %rhs
+    $w0 = COPY %res(s32)
+...
+---
+name:            test_icmp_no_canon
+body:             |
+  bb.1:
+    ; CHECK-LABEL: name: test_icmp_no_canon
+    ; CHECK: %lhs:_(s64) = COPY $x0
+    ; CHECK-NEXT: %rhs:_(s64) = G_CONSTANT i64 11
+    ; CHECK-NEXT: %res:_(s32) = G_ICMP intpred(slt), %lhs(s64), %rhs
+    ; CHECK-NEXT: $w0 = COPY %res(s32)
+    %lhs:_(s64) = COPY $x0
+    %rhs:_(s64) = G_CONSTANT i64 11
+    %res:_(s32) = G_ICMP intpred(slt), %lhs(s64), %rhs
+    $w0 = COPY %res(s32)
+...
+---
+name:            test_icmp_canon_bv
+body:             |
+  bb.1:
+    ; CHECK-LABEL: name: test_icmp_canon_bv
+    ; CHECK: %opaque1:_(s64) = COPY $x0
+    ; CHECK-NEXT: %opaque2:_(s64) = COPY $x0
+    ; CHECK-NEXT: %const1:_(s64) = G_CONSTANT i64 11
+    ; CHECK-NEXT: %const2:_(s64) = G_CONSTANT i64 12
+    ; CHECK-NEXT: %lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %const2(s64)
+    ; CHECK-NEXT: %rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %opaque2(s64)
+    ; CHECK-NEXT: %res:_(<2 x s32>) = G_ICMP intpred(sgt), %rhs(<2 x s64>), %lhs
+    ; CHECK-NEXT: $x0 = COPY %res(<2 x s32>)
+    %opaque1:_(s64) = COPY $x0
+    %opaque2:_(s64) = COPY $x0
+    %const1:_(s64) = G_CONSTANT i64 11
+    %const2:_(s64) = G_CONSTANT i64 12
+    %lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %const2(s64)
+    %rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %opaque2(s64)
+    %res:_(<2 x s32>) = G_ICMP intpred(slt), %lhs(<2 x s64>), %rhs
+    $x0 = COPY %res(<2 x s32>)
+...
+---
+name:            test_icmp_no_canon_bv_neither_const
+body:             |
+  bb.1:
+    ; CHECK-LABEL: name: test_icmp_no_canon_bv
+    ; CHECK: %opaque1:_(s64) = COPY $x0
+    ; CHECK-NEXT: %opaque2:_(s64) = COPY $x0
+    ; CHECK-NEXT: %const1:_(s64) = G_CONSTANT i64 11
+    ; CHECK-NEXT: %const2:_(s64) = G_CONSTANT i64 12
+    ; CHECK-NEXT: %lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %opaque2(s64)
+    ; CHECK-NEXT: %rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %const2(s64)
+    ; CHECK-NEXT: %res:_(<2 x s32>) = G_ICMP intpred(slt), %lhs(<2 x s64>), %rhs
+    ; CHECK-NEXT: $x0 = COPY %res(<2 x s32>)
+    %opaque1:_(s64) = COPY $x0
+    %opaque2:_(s64) = COPY $x0
+    %const1:_(s64) = G_CONSTANT i64 11
+    %const2:_(s64) = G_CONSTANT i64 12
+    %lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %opaque2(s64)
+    %rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %const2(s64)
+    %res:_(<2 x s32>) = G_ICMP intpred(slt), %lhs(<2 x s64>), %rhs
+    $x0 = COPY %res(<2 x s32>)
+...
+---
+name:            test_icmp_canon_splat
+body:             |
+  bb.1:
+    ; CHECK-LABEL: name: test_icmp_canon_splat
+    ; CHECK: %const:_(s64) = G_CONSTANT i64 11
+    ; CHECK-NEXT: %lhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR %const(s64)
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x1
+    ; CHECK-NEXT: %rhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR [[COPY]](s64)
+    ; CHECK-NEXT: %res:_(<vscale x 2 x s32>) = G_ICMP intpred(sgt), %rhs(<vscale x 2 x s64>), %lhs
+    ; CHECK-NEXT: %z:_(<vscale x 2 x s64>) = G_ZEXT %res(<vscale x 2 x s32>)
+    ; CHECK-NEXT: $z0 = COPY %z(<vscale x 2 x s64>)
+    %const:_(s64) = G_CONSTANT i64 11
+    %lhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR %const:_(s64)
+    %1:_(s64) = COPY $x1
+    %rhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR %1:_(s64)
+    %res:_(<vscale x 2 x s32>) = G_ICMP intpred(slt), %lhs(<vscale x 2 x s64>), %rhs
+    %z:_(<vscale x 2 x s64>) = G_ZEXT  %res
+    $z0 = COPY %z(<vscale x 2 x s64>)
+...
diff --git a/llvm/test/CodeGen/AArch64/arm64-ccmp.ll b/llvm/test/CodeGen/AArch64/arm64-ccmp.ll
index 50afc79a5a5768..06e957fdcc6a2a 100644
--- a/llvm/test/CodeGen/AArch64/arm64-ccmp.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-ccmp.ll
@@ -457,20 +457,12 @@ sw.bb.i.i:
 }
 
 define i64 @select_and(i32 %w0, i32 %w1, i64 %x2, i64 %x3) {
-; SDISEL-LABEL: select_and:
-; SDISEL:       ; %bb.0:
-; SDISEL-NEXT:    cmp w1, #5
-; SDISEL-NEXT:    ccmp w0, w1, #0, ne
-; SDISEL-NEXT:    csel x0, x2, x3, lt
-; SDISEL-NEXT:    ret
-;
-; GISEL-LABEL: select_and:
-; GISEL:       ; %bb.0:
-; GISEL-NEXT:    mov w8, #5 ; =0x5
-; GISEL-NEXT:    cmp w8, w1
-; GISEL-NEXT:    ccmp w0, w1, #0, ne
-; GISEL-NEXT:    csel x0, x2, x3, lt
-; GISEL-NEXT:    ret
+; CHECK-LABEL: select_and:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    cmp w1, #5
+; CHECK-NEXT:    ccmp w0, w1, #0, ne
+; CHECK-NEXT:    csel x0, x2, x3, lt
+; CHECK-NEXT:    ret
   %1 = icmp slt i32 %w0, %w1
   %2 = icmp ne i32 5, %w1
   %3 = and i1 %1, %2
@@ -479,20 +471,12 @@ define i64 @select_and(i32 %w0, i32 %w1, i64 %x2, i64 %x3) {
 }
 
 define i64 @select_or(i32 %w0, i32 %w1, i64 %x2, i64 %x3) {
-; SDISEL-LABEL: select_or:
-; SDISEL:       ; %bb.0:
-; SDISEL-NEXT:    cmp w1, #5
-; SDISEL-NEXT:    ccmp w0, w1, #8, eq
-; SDISEL-NEXT:    csel x0, x2, x3, lt
-; SDISEL-NEXT:    ret
-;
-; GISEL-LABEL: select_or:
-; GISEL:       ; %bb.0:
-; GISEL-NEXT:    mov w8, #5 ; =0x5
-; GISEL-NEXT:    cmp w8, w1
-; GISEL-NEXT:    ccmp w0, w1, #8, eq
-; GISEL-NEXT:    csel x0, x2, x3, lt
-; GISEL-NEXT:    ret
+; CHECK-LABEL: select_or:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    cmp w1, #5
+; CHECK-NEXT:    ccmp w0, w1, #8, eq
+; CHECK-NEXT:    csel x0, x2, x3, lt
+; CHECK-NEXT:    ret
   %1 = icmp slt i32 %w0, %w1
   %2 = icmp ne i32 5, %w1
   %3 = or i1 %1, %2
@@ -501,20 +485,12 @@ define i64 @select_or(i32 %w0, i32 %w1, i64 %x2, i64 %x3) {
 }
 
 define float @select_or_float(i32 %w0, i32 %w1, float %x2, float %x3) {
-; SDISEL-LABEL: select_or_float:
-; SDISEL:       ; %bb.0:
-; SDISEL-NEXT:    cmp w1, #5
-; SDISEL-NEXT:    ccmp w0, w1, #8, eq
-; SDISEL-NEXT:    fcsel s0, s0, s1, lt
-; SDISEL-NEXT:    ret
-;
-; GISEL-LABEL: select_or_float:
-; GISEL:       ; %bb.0:
-; GISEL-NEXT:    mov w8, #5 ; =0x5
-; GISEL-NEXT:    cmp w8, w1
-; GISEL-NEXT:    ccmp w0, w1, #8, eq
-; GISEL-NEXT:    fcsel s0, s0, s1, lt
-; GISEL-NEXT:    ret
+; CHECK-LABEL: select_or_float:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    cmp w1, #5
+; CHECK-NEXT:    ccmp w0, w1, #8, eq
+; CHECK-NEXT:    fcsel s0, s0, s1, lt
+; CHECK-NEXT:    ret
   %1 = icmp slt i32 %w0, %w1
   %2 = icmp ne i32 5, %w1
   %3 = or i1 %1, %2
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/fdiv.f64.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/fdiv.f64.ll
index b1cdf553b72423..0b66185d25f3e2 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/fdiv.f64.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/fdiv.f64.ll
@@ -253,7 +253,7 @@ define double @v_rcp_f64(double %x) {
 ; GFX6-NEXT:    v_div_scale_f64 v[8:9], s[4:5], 1.0, v[0:1], 1.0
 ; GFX6-NEXT:    v_rcp_f64_e32 v[4:5], v[2:3]
 ; GFX6-NEXT:    v_mov_b32_e32 v10, 0x3ff00000
-; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v10, v9
+; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v9, v10
 ; GFX6-NEXT:    v_cmp_eq_u32_e64 s[4:5], v1, v3
 ; GFX6-NEXT:    v_fma_f64 v[6:7], -v[2:3], v[4:5], 1.0
 ; GFX6-NEXT:    s_xor_b64 vcc, vcc, s[4:5]
@@ -346,7 +346,7 @@ define double @v_rcp_f64_arcp(double %x) {
 ; GFX6-NEXT:    v_div_scale_f64 v[8:9], s[4:5], 1.0, v[0:1], 1.0
 ; GFX6-NEXT:    v_rcp_f64_e32 v[4:5], v[2:3]
 ; GFX6-NEXT:    v_mov_b32_e32 v10, 0x3ff00000
-; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v10, v9
+; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v9, v10
 ; GFX6-NEXT:    v_cmp_eq_u32_e64 s[4:5], v1, v3
 ; GFX6-NEXT:    v_fma_f64 v[6:7], -v[2:3], v[4:5], 1.0
 ; GFX6-NEXT:    s_xor_b64 vcc, vcc, s[4:5]
@@ -483,7 +483,7 @@ define double @v_rcp_f64_ulp25(double %x) {
 ; GFX6-NEXT:    v_div_scale_f64 v[8:9], s[4:5], 1.0, v[0:1], 1.0
 ; GFX6-NEXT:    v_rcp_f64_e32 v[4:5], v[2:3]
 ; GFX6-NEXT:    v_mov_b32_e32 v10, 0x3ff00000
-; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v10, v9
+; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v9, v10
 ; GFX6-NEXT:    v_cmp_eq_u32_e64 s[4:5], v1, v3
 ; GFX6-NEXT:    v_fma_f64 v[6:7],...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Sep 15, 2024

@llvm/pr-subscribers-llvm-globalisel

Author: Thorsten Schütt (tschuett)

Changes

As a side-effect, we start constant folding icmps.

Patch is 35.25 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/108755.diff

11 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h (+6)
  • (modified) llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h (+10)
  • (modified) llvm/include/llvm/CodeGen/GlobalISel/Utils.h (+22)
  • (modified) llvm/include/llvm/Target/GlobalISel/Combine.td (+18-7)
  • (modified) llvm/lib/CodeGen/GlobalISel/CMakeLists.txt (+1)
  • (added) llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp (+86)
  • (modified) llvm/lib/CodeGen/GlobalISel/Utils.cpp (+40)
  • (added) llvm/test/CodeGen/AArch64/GlobalISel/combine-visit-icmp.mir (+95)
  • (modified) llvm/test/CodeGen/AArch64/arm64-ccmp.ll (+18-42)
  • (modified) llvm/test/CodeGen/AMDGPU/GlobalISel/fdiv.f64.ll (+9-9)
  • (modified) llvm/test/CodeGen/AMDGPU/rsq.f64.ll (+23-23)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
index 828532dcffb7d3..37c9422d192754 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
+#include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/Register.h"
 #include "llvm/CodeGenTypes/LowLevelType.h"
 #include "llvm/IR/InstrTypes.h"
@@ -909,6 +910,8 @@ class CombinerHelper {
   bool matchCastOfBuildVector(const MachineInstr &CastMI,
                               const MachineInstr &BVMI, BuildFnTy &MatchInfo);
 
+  bool matchCanonicalizeICmp(const MachineInstr &MI, BuildFnTy &MatchInfo);
+
 private:
   /// Checks for legality of an indexed variant of \p LdSt.
   bool isIndexedLoadStoreLegal(GLoadStore &LdSt) const;
@@ -1023,6 +1026,9 @@ class CombinerHelper {
   bool tryFoldLogicOfFCmps(GLogicalBinOp *Logic, BuildFnTy &MatchInfo);
 
   bool isCastFree(unsigned Opcode, LLT ToTy, LLT FromTy) const;
+
+  bool constantFoldICmp(const GICmp &ICmp, const GIConstant &LHSCst,
+                        const GIConstant &RHSCst, BuildFnTy &MatchInfo);
 };
 } // namespace llvm
 
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h b/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
index ef1171d9f1f64d..2c459ccdd8a731 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
@@ -950,6 +950,16 @@ class GExtOrTruncOp : public GCastOp {
   };
 };
 
+/// Represents a splat vector.
+class GSplatVector : public GenericMachineInstr {
+public:
+  Register getValueReg() const { return getOperand(1).getReg(); }
+
+  static bool classof(const MachineInstr *MI) {
+    return MI->getOpcode() == TargetOpcode::G_SPLAT_VECTOR;
+  };
+};
+
 } // namespace llvm
 
 #endif // LLVM_CODEGEN_GLOBALISEL_GENERICMACHINEINSTRS_H
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
index cf5fd6d6f288bd..4aeacfdf4ecdf0 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -593,5 +593,27 @@ bool isGuaranteedNotToBeUndef(Register Reg, const MachineRegisterInfo &MRI,
 /// estimate of the type.
 Type *getTypeForLLT(LLT Ty, LLVMContext &C);
 
+enum class GIConstantKind { Scalar, FixedVector, ScalableVector };
+
+/// An integer-like constant.
+class GIConstant {
+  GIConstantKind Kind;
+  SmallVector<APInt> Values;
+  APInt Value;
+
+public:
+  GIConstant(ArrayRef<APInt> Values)
+      : Kind(GIConstantKind::FixedVector), Values(Values) {};
+  GIConstant(const APInt &Value, GIConstantKind Kind)
+      : Kind(Kind), Value(Value) {};
+
+  GIConstantKind getKind() const { return Kind; }
+
+  APInt getScalarValue() const;
+
+  static std::optional<GIConstant> getConstant(Register Const,
+                                               const MachineRegisterInfo &MRI);
+};
+
 } // End namespace llvm.
 #endif
diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td
index a595a51d7b01ff..c66212d2ab12c8 100644
--- a/llvm/include/llvm/Target/GlobalISel/Combine.td
+++ b/llvm/include/llvm/Target/GlobalISel/Combine.td
@@ -1007,9 +1007,6 @@ def double_icmp_zero_or_combine: GICombineRule<
          (G_ICMP $root, $p, $ordst, 0))
 >;
 
-def double_icmp_zero_and_or_combine : GICombineGroup<[double_icmp_zero_and_combine,
-                                                      double_icmp_zero_or_combine]>;
-
 def and_or_disjoint_mask : GICombineRule<
   (defs root:$root, build_fn_matchinfo:$info),
   (match (wip_match_opcode G_AND):$root,
@@ -1918,6 +1915,20 @@ def cast_combines: GICombineGroup<[
   integer_of_truncate
 ]>;
 
+def canonicalize_icmp : GICombineRule<
+  (defs root:$root, build_fn_matchinfo:$matchinfo),
+  (match (G_ICMP $root, $pred, $lhs, $rhs):$cmp,
+         [{ return Helper.matchCanonicalizeICmp(*${cmp}, ${matchinfo}); }]),
+  (apply [{ Helper.applyBuildFn(*${cmp}, ${matchinfo}); }])>;
+
+def icmp_combines: GICombineGroup<[
+  canonicalize_icmp,
+  icmp_to_true_false_known_bits,
+  icmp_to_lhs_known_bits,
+  double_icmp_zero_and_combine,
+  double_icmp_zero_or_combine,
+  redundant_binop_in_equality
+]>;
 
 // FIXME: These should use the custom predicate feature once it lands.
 def undef_combines : GICombineGroup<[undef_to_fp_zero, undef_to_int_zero,
@@ -1951,7 +1962,7 @@ def const_combines : GICombineGroup<[constant_fold_fp_ops, const_ptradd_to_i2p,
 
 def known_bits_simplifications : GICombineGroup<[
   redundant_and, redundant_sext_inreg, redundant_or, urem_pow2_to_mask,
-  zext_trunc_fold, icmp_to_true_false_known_bits, icmp_to_lhs_known_bits,
+  zext_trunc_fold,
   sext_inreg_to_zext_inreg]>;
 
 def width_reduction_combines : GICombineGroup<[reduce_shl_of_extend,
@@ -1984,7 +1995,7 @@ def all_combines : GICombineGroup<[integer_reassoc_combines, trivial_combines,
     combine_extracted_vector_load,
     undef_combines, identity_combines, phi_combines,
     simplify_add_to_sub, hoist_logic_op_with_same_opcode_hands, shifts_too_big,
-    reassocs, ptr_add_immed_chain,
+    reassocs, ptr_add_immed_chain, icmp_combines,
     shl_ashr_to_sext_inreg, sext_inreg_of_load,
     width_reduction_combines, select_combines,
     known_bits_simplifications,
@@ -1998,9 +2009,9 @@ def all_combines : GICombineGroup<[integer_reassoc_combines, trivial_combines,
     constant_fold_cast_op, fabs_fneg_fold,
     intdiv_combines, mulh_combines, redundant_neg_operands,
     and_or_disjoint_mask, fma_combines, fold_binop_into_select,
-    sub_add_reg, select_to_minmax, redundant_binop_in_equality,
+    sub_add_reg, select_to_minmax, 
     fsub_to_fneg, commute_constant_to_rhs, match_ands, match_ors,
-    combine_concat_vector, double_icmp_zero_and_or_combine, match_addos,
+    combine_concat_vector, match_addos,
     sext_trunc, zext_trunc, prefer_sign_combines, combine_shuffle_concat]>;
 
 // A combine group used to for prelegalizer combiners at -O0. The combines in
diff --git a/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt b/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
index a15b76440364b1..af1717dbf76f39 100644
--- a/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
+++ b/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
@@ -7,6 +7,7 @@ add_llvm_component_library(LLVMGlobalISel
   Combiner.cpp
   CombinerHelper.cpp
   CombinerHelperCasts.cpp
+  CombinerHelperCompares.cpp
   CombinerHelperVectorOps.cpp
   GIMatchTableExecutor.cpp
   GISelChangeObserver.cpp
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp
new file mode 100644
index 00000000000000..39ac4ee205fb7a
--- /dev/null
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp
@@ -0,0 +1,86 @@
+//===- CombinerHelperCompares.cpp------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements CombinerHelper for G_ICMP.
+//
+//===----------------------------------------------------------------------===//
+#include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
+#include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
+#include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
+#include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
+#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
+#include "llvm/CodeGen/GlobalISel/Utils.h"
+#include "llvm/CodeGen/LowLevelTypeUtils.h"
+#include "llvm/CodeGen/MachineInstr.h"
+#include "llvm/CodeGen/MachineOperand.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/TargetOpcodes.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
+#include <cstdlib>
+
+#define DEBUG_TYPE "gi-combiner"
+
+using namespace llvm;
+
+bool CombinerHelper::constantFoldICmp(const GICmp &ICmp,
+                                      const GIConstant &LHSCst,
+                                      const GIConstant &RHSCst,
+                                      BuildFnTy &MatchInfo) {
+  if (LHSCst.getKind() != GIConstantKind::Scalar)
+    return false;
+
+  Register Dst = ICmp.getReg(0);
+  LLT DstTy = MRI.getType(Dst);
+
+  if (!isConstantLegalOrBeforeLegalizer(DstTy))
+    return false;
+
+  CmpInst::Predicate Pred = ICmp.getCond();
+  APInt LHS = LHSCst.getScalarValue();
+  APInt RHS = RHSCst.getScalarValue();
+
+  bool Result = ICmpInst::compare(LHS, RHS, Pred);
+
+  MatchInfo = [=](MachineIRBuilder &B) {
+    if (Result)
+      B.buildConstant(Dst, getICmpTrueVal(getTargetLowering(),
+                                          /*IsVector=*/DstTy.isVector(),
+                                          /*IsFP=*/false));
+    else
+      B.buildConstant(Dst, 0);
+  };
+
+  return true;
+}
+
+bool CombinerHelper::matchCanonicalizeICmp(const MachineInstr &MI,
+                                           BuildFnTy &MatchInfo) {
+  const GICmp *Cmp = cast<GICmp>(&MI);
+
+  Register Dst = Cmp->getReg(0);
+  Register LHS = Cmp->getLHSReg();
+  Register RHS = Cmp->getRHSReg();
+
+  CmpInst::Predicate Pred = Cmp->getCond();
+  assert(CmpInst::isIntPredicate(Pred) && "Not an integer compare!");
+  if (auto CLHS = GIConstant::getConstant(LHS, MRI)) {
+    if (auto CRHS = GIConstant::getConstant(RHS, MRI))
+      return constantFoldICmp(*Cmp, *CLHS, *CRHS, MatchInfo);
+
+    // If we have a constant, make sure it is on the RHS.
+    std::swap(LHS, RHS);
+    Pred = CmpInst::getSwappedPredicate(Pred);
+
+    MatchInfo = [=](MachineIRBuilder &B) { B.buildICmp(Pred, Dst, LHS, RHS); };
+    return true;
+  }
+
+  return false;
+}
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 1713a582d5cfe5..396f0f07ae9050 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -1968,3 +1968,43 @@ Type *llvm::getTypeForLLT(LLT Ty, LLVMContext &C) {
                            Ty.getElementCount());
   return IntegerType::get(C, Ty.getSizeInBits());
 }
+
+APInt llvm::GIConstant::getScalarValue() const {
+  assert(Kind == GIConstantKind::Scalar && "Expected scalar constant");
+
+  return Value;
+}
+
+std::optional<GIConstant>
+llvm::GIConstant::getConstant(Register Const, const MachineRegisterInfo &MRI) {
+  MachineInstr *Constant = getDefIgnoringCopies(Const, MRI);
+
+  if (GSplatVector *Splat = dyn_cast<GSplatVector>(Constant)) {
+    std::optional<ValueAndVReg> MayBeConstant =
+        getIConstantVRegValWithLookThrough(Splat->getValueReg(), MRI);
+    if (!MayBeConstant)
+      return std::nullopt;
+    return GIConstant(MayBeConstant->Value, GIConstantKind::ScalableVector);
+  }
+
+  if (GBuildVector *Build = dyn_cast<GBuildVector>(Constant)) {
+    SmallVector<APInt> Values;
+    unsigned NumSources = Build->getNumSources();
+    for (unsigned I = 0; I < NumSources; ++I) {
+      Register SrcReg = Build->getSourceReg(I);
+      std::optional<ValueAndVReg> MayBeConstant =
+          getIConstantVRegValWithLookThrough(SrcReg, MRI);
+      if (!MayBeConstant)
+        return std::nullopt;
+      Values.push_back(MayBeConstant->Value);
+    }
+    return GIConstant(Values);
+  }
+
+  std::optional<ValueAndVReg> MayBeConstant =
+      getIConstantVRegValWithLookThrough(Const, MRI);
+  if (!MayBeConstant)
+    return std::nullopt;
+
+  return GIConstant(MayBeConstant->Value, GIConstantKind::Scalar);
+}
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/combine-visit-icmp.mir b/llvm/test/CodeGen/AArch64/GlobalISel/combine-visit-icmp.mir
new file mode 100644
index 00000000000000..bf04ac02d086ab
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/combine-visit-icmp.mir
@@ -0,0 +1,95 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -o - -mtriple=aarch64-unknown-unknown -run-pass=aarch64-prelegalizer-combiner -verify-machineinstrs  %s | FileCheck %s --check-prefixes=CHECK
+
+---
+name:            test_icmp_canon
+body:             |
+  bb.1:
+    ; CHECK-LABEL: name: test_icmp_canon
+    ; CHECK: %lhs:_(s64) = G_CONSTANT i64 11
+    ; CHECK-NEXT: %rhs:_(s64) = COPY $x0
+    ; CHECK-NEXT: %res:_(s32) = G_ICMP intpred(sgt), %rhs(s64), %lhs
+    ; CHECK-NEXT: $w0 = COPY %res(s32)
+    %lhs:_(s64) = G_CONSTANT i64 11
+    %rhs:_(s64) = COPY $x0
+    %res:_(s32) = G_ICMP intpred(slt), %lhs(s64), %rhs
+    $w0 = COPY %res(s32)
+...
+---
+name:            test_icmp_no_canon
+body:             |
+  bb.1:
+    ; CHECK-LABEL: name: test_icmp_no_canon
+    ; CHECK: %lhs:_(s64) = COPY $x0
+    ; CHECK-NEXT: %rhs:_(s64) = G_CONSTANT i64 11
+    ; CHECK-NEXT: %res:_(s32) = G_ICMP intpred(slt), %lhs(s64), %rhs
+    ; CHECK-NEXT: $w0 = COPY %res(s32)
+    %lhs:_(s64) = COPY $x0
+    %rhs:_(s64) = G_CONSTANT i64 11
+    %res:_(s32) = G_ICMP intpred(slt), %lhs(s64), %rhs
+    $w0 = COPY %res(s32)
+...
+---
+name:            test_icmp_canon_bv
+body:             |
+  bb.1:
+    ; CHECK-LABEL: name: test_icmp_canon_bv
+    ; CHECK: %opaque1:_(s64) = COPY $x0
+    ; CHECK-NEXT: %opaque2:_(s64) = COPY $x0
+    ; CHECK-NEXT: %const1:_(s64) = G_CONSTANT i64 11
+    ; CHECK-NEXT: %const2:_(s64) = G_CONSTANT i64 12
+    ; CHECK-NEXT: %lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %const2(s64)
+    ; CHECK-NEXT: %rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %opaque2(s64)
+    ; CHECK-NEXT: %res:_(<2 x s32>) = G_ICMP intpred(sgt), %rhs(<2 x s64>), %lhs
+    ; CHECK-NEXT: $x0 = COPY %res(<2 x s32>)
+    %opaque1:_(s64) = COPY $x0
+    %opaque2:_(s64) = COPY $x0
+    %const1:_(s64) = G_CONSTANT i64 11
+    %const2:_(s64) = G_CONSTANT i64 12
+    %lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %const2(s64)
+    %rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %opaque2(s64)
+    %res:_(<2 x s32>) = G_ICMP intpred(slt), %lhs(<2 x s64>), %rhs
+    $x0 = COPY %res(<2 x s32>)
+...
+---
+name:            test_icmp_no_canon_bv_neither_const
+body:             |
+  bb.1:
+    ; CHECK-LABEL: name: test_icmp_no_canon_bv
+    ; CHECK: %opaque1:_(s64) = COPY $x0
+    ; CHECK-NEXT: %opaque2:_(s64) = COPY $x0
+    ; CHECK-NEXT: %const1:_(s64) = G_CONSTANT i64 11
+    ; CHECK-NEXT: %const2:_(s64) = G_CONSTANT i64 12
+    ; CHECK-NEXT: %lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %opaque2(s64)
+    ; CHECK-NEXT: %rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %const2(s64)
+    ; CHECK-NEXT: %res:_(<2 x s32>) = G_ICMP intpred(slt), %lhs(<2 x s64>), %rhs
+    ; CHECK-NEXT: $x0 = COPY %res(<2 x s32>)
+    %opaque1:_(s64) = COPY $x0
+    %opaque2:_(s64) = COPY $x0
+    %const1:_(s64) = G_CONSTANT i64 11
+    %const2:_(s64) = G_CONSTANT i64 12
+    %lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %opaque2(s64)
+    %rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %const2(s64)
+    %res:_(<2 x s32>) = G_ICMP intpred(slt), %lhs(<2 x s64>), %rhs
+    $x0 = COPY %res(<2 x s32>)
+...
+---
+name:            test_icmp_canon_splat
+body:             |
+  bb.1:
+    ; CHECK-LABEL: name: test_icmp_canon_splat
+    ; CHECK: %const:_(s64) = G_CONSTANT i64 11
+    ; CHECK-NEXT: %lhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR %const(s64)
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x1
+    ; CHECK-NEXT: %rhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR [[COPY]](s64)
+    ; CHECK-NEXT: %res:_(<vscale x 2 x s32>) = G_ICMP intpred(sgt), %rhs(<vscale x 2 x s64>), %lhs
+    ; CHECK-NEXT: %z:_(<vscale x 2 x s64>) = G_ZEXT %res(<vscale x 2 x s32>)
+    ; CHECK-NEXT: $z0 = COPY %z(<vscale x 2 x s64>)
+    %const:_(s64) = G_CONSTANT i64 11
+    %lhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR %const:_(s64)
+    %1:_(s64) = COPY $x1
+    %rhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR %1:_(s64)
+    %res:_(<vscale x 2 x s32>) = G_ICMP intpred(slt), %lhs(<vscale x 2 x s64>), %rhs
+    %z:_(<vscale x 2 x s64>) = G_ZEXT  %res
+    $z0 = COPY %z(<vscale x 2 x s64>)
+...
diff --git a/llvm/test/CodeGen/AArch64/arm64-ccmp.ll b/llvm/test/CodeGen/AArch64/arm64-ccmp.ll
index 50afc79a5a5768..06e957fdcc6a2a 100644
--- a/llvm/test/CodeGen/AArch64/arm64-ccmp.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-ccmp.ll
@@ -457,20 +457,12 @@ sw.bb.i.i:
 }
 
 define i64 @select_and(i32 %w0, i32 %w1, i64 %x2, i64 %x3) {
-; SDISEL-LABEL: select_and:
-; SDISEL:       ; %bb.0:
-; SDISEL-NEXT:    cmp w1, #5
-; SDISEL-NEXT:    ccmp w0, w1, #0, ne
-; SDISEL-NEXT:    csel x0, x2, x3, lt
-; SDISEL-NEXT:    ret
-;
-; GISEL-LABEL: select_and:
-; GISEL:       ; %bb.0:
-; GISEL-NEXT:    mov w8, #5 ; =0x5
-; GISEL-NEXT:    cmp w8, w1
-; GISEL-NEXT:    ccmp w0, w1, #0, ne
-; GISEL-NEXT:    csel x0, x2, x3, lt
-; GISEL-NEXT:    ret
+; CHECK-LABEL: select_and:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    cmp w1, #5
+; CHECK-NEXT:    ccmp w0, w1, #0, ne
+; CHECK-NEXT:    csel x0, x2, x3, lt
+; CHECK-NEXT:    ret
   %1 = icmp slt i32 %w0, %w1
   %2 = icmp ne i32 5, %w1
   %3 = and i1 %1, %2
@@ -479,20 +471,12 @@ define i64 @select_and(i32 %w0, i32 %w1, i64 %x2, i64 %x3) {
 }
 
 define i64 @select_or(i32 %w0, i32 %w1, i64 %x2, i64 %x3) {
-; SDISEL-LABEL: select_or:
-; SDISEL:       ; %bb.0:
-; SDISEL-NEXT:    cmp w1, #5
-; SDISEL-NEXT:    ccmp w0, w1, #8, eq
-; SDISEL-NEXT:    csel x0, x2, x3, lt
-; SDISEL-NEXT:    ret
-;
-; GISEL-LABEL: select_or:
-; GISEL:       ; %bb.0:
-; GISEL-NEXT:    mov w8, #5 ; =0x5
-; GISEL-NEXT:    cmp w8, w1
-; GISEL-NEXT:    ccmp w0, w1, #8, eq
-; GISEL-NEXT:    csel x0, x2, x3, lt
-; GISEL-NEXT:    ret
+; CHECK-LABEL: select_or:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    cmp w1, #5
+; CHECK-NEXT:    ccmp w0, w1, #8, eq
+; CHECK-NEXT:    csel x0, x2, x3, lt
+; CHECK-NEXT:    ret
   %1 = icmp slt i32 %w0, %w1
   %2 = icmp ne i32 5, %w1
   %3 = or i1 %1, %2
@@ -501,20 +485,12 @@ define i64 @select_or(i32 %w0, i32 %w1, i64 %x2, i64 %x3) {
 }
 
 define float @select_or_float(i32 %w0, i32 %w1, float %x2, float %x3) {
-; SDISEL-LABEL: select_or_float:
-; SDISEL:       ; %bb.0:
-; SDISEL-NEXT:    cmp w1, #5
-; SDISEL-NEXT:    ccmp w0, w1, #8, eq
-; SDISEL-NEXT:    fcsel s0, s0, s1, lt
-; SDISEL-NEXT:    ret
-;
-; GISEL-LABEL: select_or_float:
-; GISEL:       ; %bb.0:
-; GISEL-NEXT:    mov w8, #5 ; =0x5
-; GISEL-NEXT:    cmp w8, w1
-; GISEL-NEXT:    ccmp w0, w1, #8, eq
-; GISEL-NEXT:    fcsel s0, s0, s1, lt
-; GISEL-NEXT:    ret
+; CHECK-LABEL: select_or_float:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    cmp w1, #5
+; CHECK-NEXT:    ccmp w0, w1, #8, eq
+; CHECK-NEXT:    fcsel s0, s0, s1, lt
+; CHECK-NEXT:    ret
   %1 = icmp slt i32 %w0, %w1
   %2 = icmp ne i32 5, %w1
   %3 = or i1 %1, %2
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/fdiv.f64.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/fdiv.f64.ll
index b1cdf553b72423..0b66185d25f3e2 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/fdiv.f64.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/fdiv.f64.ll
@@ -253,7 +253,7 @@ define double @v_rcp_f64(double %x) {
 ; GFX6-NEXT:    v_div_scale_f64 v[8:9], s[4:5], 1.0, v[0:1], 1.0
 ; GFX6-NEXT:    v_rcp_f64_e32 v[4:5], v[2:3]
 ; GFX6-NEXT:    v_mov_b32_e32 v10, 0x3ff00000
-; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v10, v9
+; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v9, v10
 ; GFX6-NEXT:    v_cmp_eq_u32_e64 s[4:5], v1, v3
 ; GFX6-NEXT:    v_fma_f64 v[6:7], -v[2:3], v[4:5], 1.0
 ; GFX6-NEXT:    s_xor_b64 vcc, vcc, s[4:5]
@@ -346,7 +346,7 @@ define double @v_rcp_f64_arcp(double %x) {
 ; GFX6-NEXT:    v_div_scale_f64 v[8:9], s[4:5], 1.0, v[0:1], 1.0
 ; GFX6-NEXT:    v_rcp_f64_e32 v[4:5], v[2:3]
 ; GFX6-NEXT:    v_mov_b32_e32 v10, 0x3ff00000
-; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v10, v9
+; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v9, v10
 ; GFX6-NEXT:    v_cmp_eq_u32_e64 s[4:5], v1, v3
 ; GFX6-NEXT:    v_fma_f64 v[6:7], -v[2:3], v[4:5], 1.0
 ; GFX6-NEXT:    s_xor_b64 vcc, vcc, s[4:5]
@@ -483,7 +483,7 @@ define double @v_rcp_f64_ulp25(double %x) {
 ; GFX6-NEXT:    v_div_scale_f64 v[8:9], s[4:5], 1.0, v[0:1], 1.0
 ; GFX6-NEXT:    v_rcp_f64_e32 v[4:5], v[2:3]
 ; GFX6-NEXT:    v_mov_b32_e32 v10, 0x3ff00000
-; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v10, v9
+; GFX6-NEXT:    v_cmp_eq_u32_e32 vcc, v9, v10
 ; GFX6-NEXT:    v_cmp_eq_u32_e64 s[4:5], v1, v3
 ; GFX6-NEXT:    v_fma_f64 v[6:7],...
[truncated]

arsenm
arsenm reviewed Sep 15, 2024
View reviewed changes
llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp
std::swap(LHS, RHS);
Pred = CmpInst::getSwappedPredicate(Pred);

MatchInfo = [=](MachineIRBuilder &B) { B.buildICmp(Pred, Dst, LHS, RHS); };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also mutate the instruction in place without creating a new one

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never do that!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never do that!

That's not a real answer to a review suggestion. If it's easy to mutate in place we should generally try to do that first. The idea is that it's cheaper to do that than creating a new MachineInstr and the operands. For niche optimizations it's not a big deal, but it's a nice to have.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! But I don't like this low-level manipulation of instructions. It is error prone. I prefer the build and erase pattern.

llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp
Register RHS = Cmp->getRHSReg();

CmpInst::Predicate Pred = Cmp->getCond();
assert(CmpInst::isIntPredicate(Pred) && "Not an integer compare!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also handle fcmp, you don't need to know what the constant kind or value is, just that it's a constant

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be a different PR. This PR has one pattern that matches G_ICMP. We would need a different pattern for G_FCMP. This PR introduces GIConstant, which is a wrapper around a vector of APInt. For G_FCMP, we would need to introduce GFConstant, which is a wrapper around a vector of APFloat.

This PR canonicalizes and constant folds icmps.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a split out from #105991. It only handles icmps. We could add the same for fcmps, but the market share is probably lower without having data.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we can always generalize this later.

llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp

using namespace llvm;

bool CombinerHelper::constantFoldICmp(const GICmp &ICmp,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not put this in the builder like other constant folds?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The builder only folds icmps that it builds. This PR adds one combine that matches G_ICMP. While running the combiner, we could find G_ICMPs where the parameters are constants due to combining. The builder would never build this G_ICMP again. In DAG terms, this is visitICmp.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have ConstantFoldICmp, but I needed this on top of GIConstants.

llvm-project/llvm/lib/CodeGen/GlobalISel/Utils.cpp

Line 1013 in 76b54df

llvm::ConstantFoldICmp(unsigned Pred, const Register Op1, const Register Op2,

This used by the builder.

aemerson
aemerson reviewed Sep 16, 2024
View reviewed changes
llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp
Register RHS = Cmp->getRHSReg();

CmpInst::Predicate Pred = Cmp->getCond();
assert(CmpInst::isIntPredicate(Pred) && "Not an integer compare!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we can always generalize this later.

llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp
std::swap(LHS, RHS);
Pred = CmpInst::getSwappedPredicate(Pred);

MatchInfo = [=](MachineIRBuilder &B) { B.buildICmp(Pred, Dst, LHS, RHS); };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never do that!

That's not a real answer to a review suggestion. If it's easy to mutate in place we should generally try to do that first. The idea is that it's cheaper to do that than creating a new MachineInstr and the operands. For niche optimizations it's not a big deal, but it's a nice to have.

llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -593,5 +593,27 @@ bool isGuaranteedNotToBeUndef(Register Reg, const MachineRegisterInfo &MRI,
/// estimate of the type.
Type *getTypeForLLT(LLT Ty, LLVMContext &C);

enum class GIConstantKind { Scalar, FixedVector, ScalableVector };

/// An integer-like constant.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class could do with more documentation about its intended purpose, in what situation it's supposed to be used.

aemerson
aemerson approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@aemerson aemerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I'm reserving the right to change the matchers to mutate the instruction in future.

@tschuett tschuett merged commit 5c348f6 into llvm:main Sep 16, 2024
8 checks passed
@tschuett tschuett deleted the gisel-canonicalize_icmp branch September 16, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arsenm arsenm arsenm left review comments

@aemerson aemerson aemerson approved these changes

Assignees
No one assigned
Labels
backend:AArch64 backend:AMDGPU llvm:globalisel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tschuett @llvmbot @aemerson @arsenm