Skip to content
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.

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

[GlobalIsel] Combine select to integer min max more #92570

Merged
merged 2 commits into from
May 18, 2024

Conversation

tschuett
Copy link
Member

No description provided.

@llvmbot
Copy link
Collaborator

llvmbot commented May 17, 2024

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-aarch64

Author: Thorsten Schütt (tschuett)

Changes

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

3 Files Affected:

  • (modified) llvm/include/llvm/Target/GlobalISel/Combine.td (+1-1)
  • (modified) llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp (+50-34)
  • (modified) llvm/test/CodeGen/AArch64/GlobalISel/combine-select.mir (+1-2)
diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td
index a1d39d910bc8e..5d4b5a2479f6a 100644
--- a/llvm/include/llvm/Target/GlobalISel/Combine.td
+++ b/llvm/include/llvm/Target/GlobalISel/Combine.td
@@ -1309,7 +1309,7 @@ def select_to_minmax: GICombineRule<
 
 def select_to_iminmax: GICombineRule<
   (defs root:$root, build_fn_matchinfo:$info),
-  (match (G_ICMP $tst, $tst1, $x, $y),
+  (match (G_ICMP $tst, $tst1, $a, $b),
          (G_SELECT $root, $tst, $x, $y),
          [{ return Helper.matchSelectIMinMax(${root}, ${info}); }]),
   (apply [{ Helper.applyBuildFnMO(${root}, ${info}); }])>;
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index a87a26d72b452..0d2e59541b05a 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -6772,46 +6772,62 @@ bool CombinerHelper::matchSelectIMinMax(const MachineOperand &MO,
   if (CmpInst::isEquality(Pred))
     return false;
 
-  [[maybe_unused]] Register CmpLHS = Cmp->getLHSReg();
-  [[maybe_unused]] Register CmpRHS = Cmp->getRHSReg();
+  Register CmpLHS = Cmp->getLHSReg();
+  Register CmpRHS = Cmp->getRHSReg();
+
+  // We can swap CmpLHS and CmpRHS for higher hitrate.
+  if (True == CmpRHS && False == CmpLHS) {
+    std::swap(CmpLHS, CmpRHS);
+    Pred = CmpInst::getSwappedPredicate(Pred);
+  }
 
   // (icmp X, Y) ? X : Y -> integer minmax.
   // see matchSelectPattern in ValueTracking.
   // Legality between G_SELECT and integer minmax can differ.
-  assert(True == CmpLHS && False == CmpRHS && "unexpected MIR pattern");
-
-  switch (Pred) {
-  case ICmpInst::ICMP_UGT:
-  case ICmpInst::ICMP_UGE: {
-    if (!isLegalOrBeforeLegalizer({TargetOpcode::G_UMAX, DstTy}))
-      return false;
-    MatchInfo = [=](MachineIRBuilder &B) { B.buildUMax(DstReg, True, False); };
-    return true;
-  }
-  case ICmpInst::ICMP_SGT:
-  case ICmpInst::ICMP_SGE: {
-    if (!isLegalOrBeforeLegalizer({TargetOpcode::G_SMAX, DstTy}))
-      return false;
-    MatchInfo = [=](MachineIRBuilder &B) { B.buildSMax(DstReg, True, False); };
-    return true;
-  }
-  case ICmpInst::ICMP_ULT:
-  case ICmpInst::ICMP_ULE: {
-    if (!isLegalOrBeforeLegalizer({TargetOpcode::G_UMIN, DstTy}))
-      return false;
-    MatchInfo = [=](MachineIRBuilder &B) { B.buildUMin(DstReg, True, False); };
-    return true;
-  }
-  case ICmpInst::ICMP_SLT:
-  case ICmpInst::ICMP_SLE: {
-    if (!isLegalOrBeforeLegalizer({TargetOpcode::G_SMIN, DstTy}))
+  if (True == CmpLHS && False == CmpRHS) {
+    switch (Pred) {
+    case ICmpInst::ICMP_UGT:
+    case ICmpInst::ICMP_UGE: {
+      if (!isLegalOrBeforeLegalizer({TargetOpcode::G_UMAX, DstTy}))
+        return false;
+      MatchInfo = [=](MachineIRBuilder &B) {
+        B.buildUMax(DstReg, True, False);
+      };
+      return true;
+    }
+    case ICmpInst::ICMP_SGT:
+    case ICmpInst::ICMP_SGE: {
+      if (!isLegalOrBeforeLegalizer({TargetOpcode::G_SMAX, DstTy}))
+        return false;
+      MatchInfo = [=](MachineIRBuilder &B) {
+        B.buildSMax(DstReg, True, False);
+      };
+      return true;
+    }
+    case ICmpInst::ICMP_ULT:
+    case ICmpInst::ICMP_ULE: {
+      if (!isLegalOrBeforeLegalizer({TargetOpcode::G_UMIN, DstTy}))
+        return false;
+      MatchInfo = [=](MachineIRBuilder &B) {
+        B.buildUMin(DstReg, True, False);
+      };
+      return true;
+    }
+    case ICmpInst::ICMP_SLT:
+    case ICmpInst::ICMP_SLE: {
+      if (!isLegalOrBeforeLegalizer({TargetOpcode::G_SMIN, DstTy}))
+        return false;
+      MatchInfo = [=](MachineIRBuilder &B) {
+        B.buildSMin(DstReg, True, False);
+      };
+      return true;
+    }
+    default:
       return false;
-    MatchInfo = [=](MachineIRBuilder &B) { B.buildSMin(DstReg, True, False); };
-    return true;
-  }
-  default:
-    return false;
+    }
   }
+
+  return false;
 }
 
 bool CombinerHelper::matchSelect(MachineInstr &MI, BuildFnTy &MatchInfo) {
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/combine-select.mir b/llvm/test/CodeGen/AArch64/GlobalISel/combine-select.mir
index 8d9ad8d7cacaf..353c1550d6974 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/combine-select.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/combine-select.mir
@@ -911,8 +911,7 @@ body:             |
     ; CHECK-NEXT: %f1:_(s32) = G_TRUNC [[COPY1]](s64)
     ; CHECK-NEXT: %t:_(<4 x s32>) = G_BUILD_VECTOR %t1(s32), %t1(s32), %t1(s32), %t1(s32)
     ; CHECK-NEXT: %f:_(<4 x s32>) = G_BUILD_VECTOR %f1(s32), %f1(s32), %f1(s32), %f1(s32)
-    ; CHECK-NEXT: %c:_(<4 x s32>) = G_ICMP intpred(sle), %f(<4 x s32>), %t
-    ; CHECK-NEXT: %sel:_(<4 x s32>) = exact G_SELECT %c(<4 x s32>), %t, %f
+    ; CHECK-NEXT: %sel:_(<4 x s32>) = G_SMAX %t, %f
     ; CHECK-NEXT: $q0 = COPY %sel(<4 x s32>)
     %0:_(s64) = COPY $x0
     %1:_(s64) = COPY $x1

Copy link
Contributor

@jayfoad jayfoad left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp Outdated Show resolved Hide resolved
@tschuett tschuett merged commit 778826f into llvm:main May 18, 2024
4 checks passed
@tschuett tschuett deleted the gisel-select-integer-max2 branch May 18, 2024 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants