Skip to content

Conversation

@dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Feb 1, 2025

This patch extends f6bb156 to handle minmax intrinsics.
Motivating case: https://alive2.llvm.org/ce/z/JFKbYn
Addresses a regression caused by #121958.

It also works for *.sat. But no real-world benefit is observed.

@dtcxzyw dtcxzyw requested a review from goldsteinn February 1, 2025 15:20
@dtcxzyw dtcxzyw requested a review from nikic as a code owner February 1, 2025 15:20
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Feb 1, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 1, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch extends f6bb156 to handle minmax intrinsics.
Motivating case: https://alive2.llvm.org/ce/z/JFKbYn
Addresses a regression caused by #121958.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp (+24-12)
  • (modified) llvm/test/Transforms/InstCombine/select-min-max.ll (+63)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 29c5cef84ccdb7..382078e85a17b6 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -1771,26 +1771,38 @@ static Value *foldSelectInstWithICmpConst(SelectInst &SI, ICmpInst *ICI,
       return Builder.CreateBinaryIntrinsic(Intrinsic::smin, V, TVal);
   }
 
-  BinaryOperator *BO;
+  // Fold icmp(X) ? f(X) : C to f(X) when f(X) is guaranteed to be equal to C
+  // for all X in the exact range of the inverse predicate.
+  Instruction *Op;
   const APInt *C;
   CmpInst::Predicate CPred;
-  if (match(&SI, m_Select(m_Specific(ICI), m_APInt(C), m_BinOp(BO))))
+  if (match(&SI, m_Select(m_Specific(ICI), m_APInt(C), m_Instruction(Op))))
     CPred = ICI->getPredicate();
-  else if (match(&SI, m_Select(m_Specific(ICI), m_BinOp(BO), m_APInt(C))))
+  else if (match(&SI, m_Select(m_Specific(ICI), m_Instruction(Op), m_APInt(C))))
     CPred = ICI->getInversePredicate();
   else
     return nullptr;
 
-  const APInt *BinOpC;
-  if (!match(BO, m_BinOp(m_Specific(V), m_APInt(BinOpC))))
-    return nullptr;
-
-  ConstantRange R = ConstantRange::makeExactICmpRegion(CPred, *CmpC)
-                        .binaryOp(BO->getOpcode(), *BinOpC);
-  if (R == *C) {
-    BO->dropPoisonGeneratingFlags();
-    return BO;
+  ConstantRange InvDomCR = ConstantRange::makeExactICmpRegion(CPred, *CmpC);
+  const APInt *OpC;
+  if (match(Op, m_BinOp(m_Specific(V), m_APInt(OpC)))) {
+    ConstantRange R = InvDomCR.binaryOp(
+        static_cast<Instruction::BinaryOps>(Op->getOpcode()), *OpC);
+    if (R == *C) {
+      Op->dropPoisonGeneratingFlags();
+      return Op;
+    }
+  }
+  if (auto *MMI = dyn_cast<MinMaxIntrinsic>(Op);
+      MMI && MMI->getLHS() == V && match(MMI->getRHS(), m_APInt(OpC))) {
+    ConstantRange R = ConstantRange::intrinsic(MMI->getIntrinsicID(),
+                                               {InvDomCR, ConstantRange(*OpC)});
+    if (R == *C) {
+      MMI->dropPoisonGeneratingAnnotations();
+      return MMI;
+    }
   }
+
   return nullptr;
 }
 
diff --git a/llvm/test/Transforms/InstCombine/select-min-max.ll b/llvm/test/Transforms/InstCombine/select-min-max.ll
index f13223284e6a68..0430fcd5ad3700 100644
--- a/llvm/test/Transforms/InstCombine/select-min-max.ll
+++ b/llvm/test/Transforms/InstCombine/select-min-max.ll
@@ -301,3 +301,66 @@ define i8 @not_smin_swap(i8 %i41, i8 %i43) {
   %spec.select = select i1 %i44, i8 %i46, i8 0
   ret i8 %spec.select
 }
+
+define i8 @sel_umin_constant(i8 %x) {
+; CHECK-LABEL: @sel_umin_constant(
+; CHECK-NEXT:    [[UMIN:%.*]] = call i8 @llvm.umin.i8(i8 [[X:%.*]], i8 16)
+; CHECK-NEXT:    ret i8 [[UMIN]]
+;
+  %cmp = icmp sgt i8 %x, -1
+  %umin = call i8 @llvm.umin.i8(i8 %x, i8 16)
+  %sel = select i1 %cmp, i8 %umin, i8 16
+  ret i8 %sel
+}
+
+define i8 @sel_constant_smax_with_range_attr(i8 %x) {
+; CHECK-LABEL: @sel_constant_smax_with_range_attr(
+; CHECK-NEXT:    [[SEL:%.*]] = call i8 @llvm.smax.i8(i8 [[X:%.*]], i8 16)
+; CHECK-NEXT:    ret i8 [[SEL]]
+;
+  %cmp = icmp slt i8 %x, 0
+  %smax = call range(i8 8, 16) i8 @llvm.smax.i8(i8 %x, i8 16)
+  %sel = select i1 %cmp, i8 16, i8 %smax
+  ret i8 %sel
+}
+
+; Negative tests
+
+define i8 @sel_umin_constant_mismatch(i8 %x) {
+; CHECK-LABEL: @sel_umin_constant_mismatch(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], -1
+; CHECK-NEXT:    [[UMIN:%.*]] = call i8 @llvm.umin.i8(i8 [[X]], i8 16)
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[CMP]], i8 [[UMIN]], i8 15
+; CHECK-NEXT:    ret i8 [[SEL]]
+;
+  %cmp = icmp sgt i8 %x, -1
+  %umin = call i8 @llvm.umin.i8(i8 %x, i8 16)
+  %sel = select i1 %cmp, i8 %umin, i8 15
+  ret i8 %sel
+}
+
+define i8 @sel_umin_constant_op_mismatch(i8 %x, i8 %y) {
+; CHECK-LABEL: @sel_umin_constant_op_mismatch(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], -1
+; CHECK-NEXT:    [[UMIN:%.*]] = call i8 @llvm.umin.i8(i8 [[Y:%.*]], i8 16)
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[CMP]], i8 [[UMIN]], i8 16
+; CHECK-NEXT:    ret i8 [[SEL]]
+;
+  %cmp = icmp sgt i8 %x, -1
+  %umin = call i8 @llvm.umin.i8(i8 %y, i8 16)
+  %sel = select i1 %cmp, i8 %umin, i8 16
+  ret i8 %sel
+}
+
+define i8 @sel_umin_non_constant(i8 %x, i8 %y) {
+; CHECK-LABEL: @sel_umin_non_constant(
+; CHECK-NEXT:    [[UMIN:%.*]] = call i8 @llvm.umin.i8(i8 [[X:%.*]], i8 16)
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp slt i8 [[X]], 0
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[CMP1]], i8 [[Y:%.*]], i8 [[UMIN]]
+; CHECK-NEXT:    ret i8 [[SEL]]
+;
+  %cmp = icmp sgt i8 %x, -1
+  %umin = call i8 @llvm.umin.i8(i8 %x, i8 16)
+  %sel = select i1 %cmp, i8 %umin, i8 %y
+  ret i8 %sel
+}

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@dtcxzyw dtcxzyw merged commit caeefe7 into llvm:main Feb 2, 2025
9 of 11 checks passed
@dtcxzyw dtcxzyw deleted the perf/select-minmax-c branch February 2, 2025 09:05
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 2, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building llvm at step 7 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/12880

Here is the relevant piece of the build log for the reference
Step 7 (Add check check-offload) failure: test (failure)
******************** TEST 'libomptarget :: amdgcn-amd-amdhsa :: mapping/declare_mapper_nested_mappers.cpp' FAILED ********************
Exit Code: 2

Command Output (stdout):
--
# RUN: at line 1
/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/clang++ -fopenmp    -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib  -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/mapping/declare_mapper_nested_mappers.cpp -o /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/mapping/Output/declare_mapper_nested_mappers.cpp.tmp -Xoffload-linker -lc -Xoffload-linker -lm /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a && /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/mapping/Output/declare_mapper_nested_mappers.cpp.tmp | /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/mapping/declare_mapper_nested_mappers.cpp
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/clang++ -fopenmp -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/mapping/declare_mapper_nested_mappers.cpp -o /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/mapping/Output/declare_mapper_nested_mappers.cpp.tmp -Xoffload-linker -lc -Xoffload-linker -lm /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/mapping/Output/declare_mapper_nested_mappers.cpp.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/mapping/declare_mapper_nested_mappers.cpp
# .---command stderr------------
# | FileCheck error: '<stdin>' is empty.
# | FileCheck command line:  /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/mapping/declare_mapper_nested_mappers.cpp
# `-----------------------------
# error: command failed with exit status: 2

--

********************


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants