[llvm] r295758 - [InstCombine] canonicalize non-obivous forms of integer min/max
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 21 11:33:53 PST 2017
Author: spatel
Date: Tue Feb 21 13:33:53 2017
New Revision: 295758
URL: http://llvm.org/viewvc/llvm-project?rev=295758&view=rev
Log:
[InstCombine] canonicalize non-obivous forms of integer min/max
This is part of trying to clean up our handling of min/max patterns in IR.
By converting these to canonical form, we're more likely to recognize them
because there are various places in InstCombine that don't use
matchSelectPattern or m_SMax and friends.
The backend fixups referenced in the now deleted TODO comment were added with:
https://reviews.llvm.org/rL291392
https://reviews.llvm.org/rL289738
If there's any codegen fallout from this change, we should be able to address
it in DAGCombiner or target-specific lowering.
Modified:
llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
llvm/trunk/test/Transforms/InstCombine/select.ll
llvm/trunk/test/Transforms/InstCombine/select_meta.ll
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp?rev=295758&r1=295757&r2=295758&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp Tue Feb 21 13:33:53 2017
@@ -499,18 +499,16 @@ static bool adjustMinMax(SelectInst &Sel
return true;
}
-/// If this is an integer min/max where the select's 'true' operand is a
-/// constant, canonicalize that constant to the 'false' operand:
-/// select (icmp Pred X, C), C, X --> select (icmp Pred' X, C), X, C
+/// If this is an integer min/max (icmp + select) with a constant operand,
+/// create the canonical icmp for the min/max operation and canonicalize the
+/// constant to the 'false' operand of the select:
+/// select (icmp Pred X, C1), C2, X --> select (icmp Pred' X, C2), X, C2
+/// Note: if C1 != C2, this will change the icmp constant to the existing
+/// constant operand of the select.
static Instruction *
canonicalizeMinMaxWithConstant(SelectInst &Sel, ICmpInst &Cmp,
InstCombiner::BuilderTy &Builder) {
- // TODO: We should also canonicalize min/max when the select has a different
- // constant value than the cmp constant, but we need to fix the backend first.
- if (!Cmp.hasOneUse() || !isa<Constant>(Cmp.getOperand(1)) ||
- !isa<Constant>(Sel.getTrueValue()) ||
- isa<Constant>(Sel.getFalseValue()) ||
- Cmp.getOperand(1) != Sel.getTrueValue())
+ if (!Cmp.hasOneUse() || !isa<Constant>(Cmp.getOperand(1)))
return nullptr;
// Canonicalize the compare predicate based on whether we have min or max.
@@ -525,16 +523,25 @@ canonicalizeMinMaxWithConstant(SelectIns
default: return nullptr;
}
- // Canonicalize the constant to the right side.
- if (isa<Constant>(LHS))
- std::swap(LHS, RHS);
-
- Value *NewCmp = Builder.CreateICmp(NewPred, LHS, RHS);
- SelectInst *NewSel = SelectInst::Create(NewCmp, LHS, RHS, "", nullptr, &Sel);
-
- // We swapped the select operands, so swap the metadata too.
- NewSel->swapProfMetadata();
- return NewSel;
+ // Is this already canonical?
+ if (Cmp.getOperand(0) == LHS && Cmp.getOperand(1) == RHS &&
+ Cmp.getPredicate() == NewPred)
+ return nullptr;
+
+ // Create the canonical compare and plug it into the select.
+ Sel.setCondition(Builder.CreateICmp(NewPred, LHS, RHS));
+
+ // If the select operands did not change, we're done.
+ if (Sel.getTrueValue() == LHS && Sel.getFalseValue() == RHS)
+ return &Sel;
+
+ // If we are swapping the select operands, swap the metadata too.
+ assert(Sel.getTrueValue() == RHS && Sel.getFalseValue() == LHS &&
+ "Unexpected results from matchSelectPattern");
+ Sel.setTrueValue(LHS);
+ Sel.setFalseValue(RHS);
+ Sel.swapProfMetadata();
+ return &Sel;
}
/// Visit a SelectInst that has an ICmpInst as its first operand.
Modified: llvm/trunk/test/Transforms/InstCombine/select.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/select.ll?rev=295758&r1=295757&r2=295758&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/select.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/select.ll Tue Feb 21 13:33:53 2017
@@ -1264,11 +1264,10 @@ define i32 @PR23757(i32 %x) {
define i32 @PR27137(i32 %a) {
; CHECK-LABEL: @PR27137(
; CHECK-NEXT: [[NOT_A:%.*]] = xor i32 %a, -1
-; CHECK-NEXT: [[C0:%.*]] = icmp slt i32 %a, 0
+; CHECK-NEXT: [[C0:%.*]] = icmp sgt i32 [[NOT_A]], -1
; CHECK-NEXT: [[S0:%.*]] = select i1 [[C0]], i32 [[NOT_A]], i32 -1
; CHECK-NEXT: ret i32 [[S0]]
;
-
%not_a = xor i32 %a, -1
%c0 = icmp slt i32 %a, 0
%s0 = select i1 %c0, i32 %not_a, i32 -1
Modified: llvm/trunk/test/Transforms/InstCombine/select_meta.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/select_meta.ll?rev=295758&r1=295757&r2=295758&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/select_meta.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/select_meta.ll Tue Feb 21 13:33:53 2017
@@ -193,12 +193,11 @@ define i32 @test74(i32 %x) {
ret i32 %retval
}
-; FIXME:
; The compare should change, but the metadata remains the same because the select operands are not swapped.
define i32 @smin1(i32 %x) {
; CHECK-LABEL: @smin1(
; CHECK-NEXT: [[NOT_X:%.*]] = xor i32 %x, -1
-; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 %x, 0
+; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[NOT_X]], -1
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 [[NOT_X]], i32 -1, !prof ![[MD1]]
; CHECK-NEXT: ret i32 [[SEL]]
;
@@ -208,13 +207,12 @@ define i32 @smin1(i32 %x) {
ret i32 %sel
}
-; FIXME:
; The compare should change, and the metadata is swapped because the select operands are swapped.
define i32 @smin2(i32 %x) {
; CHECK-LABEL: @smin2(
; CHECK-NEXT: [[NOT_X:%.*]] = xor i32 %x, -1
-; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 %x, 0
-; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 -1, i32 [[NOT_X]], !prof ![[MD1]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[NOT_X]], -1
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 [[NOT_X]], i32 -1, !prof ![[MD3]]
; CHECK-NEXT: ret i32 [[SEL]]
;
%not_x = xor i32 %x, -1
@@ -223,12 +221,11 @@ define i32 @smin2(i32 %x) {
ret i32 %sel
}
-; FIXME:
; The compare should change, but the metadata remains the same because the select operands are not swapped.
define i32 @smax1(i32 %x) {
; CHECK-LABEL: @smax1(
; CHECK-NEXT: [[NOT_X:%.*]] = xor i32 %x, -1
-; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 %x, 0
+; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 [[NOT_X]], -1
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 [[NOT_X]], i32 -1, !prof ![[MD1]]
; CHECK-NEXT: ret i32 [[SEL]]
;
@@ -238,13 +235,12 @@ define i32 @smax1(i32 %x) {
ret i32 %sel
}
-; FIXME:
; The compare should change, and the metadata is swapped because the select operands are swapped.
define i32 @smax2(i32 %x) {
; CHECK-LABEL: @smax2(
; CHECK-NEXT: [[NOT_X:%.*]] = xor i32 %x, -1
-; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 %x, 0
-; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 -1, i32 [[NOT_X]], !prof ![[MD1]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 [[NOT_X]], -1
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 [[NOT_X]], i32 -1, !prof ![[MD3]]
; CHECK-NEXT: ret i32 [[SEL]]
;
%not_x = xor i32 %x, -1
@@ -253,11 +249,10 @@ define i32 @smax2(i32 %x) {
ret i32 %sel
}
-; FIXME:
; The compare should change, but the metadata remains the same because the select operands are not swapped.
define i32 @umin1(i32 %x) {
; CHECK-LABEL: @umin1(
-; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 %x, -1
+; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 %x, -2147483648
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 %x, i32 -2147483648, !prof ![[MD1]]
; CHECK-NEXT: ret i32 [[SEL]]
;
@@ -266,12 +261,11 @@ define i32 @umin1(i32 %x) {
ret i32 %sel
}
-; FIXME:
; The compare should change, and the metadata is swapped because the select operands are swapped.
define i32 @umin2(i32 %x) {
; CHECK-LABEL: @umin2(
-; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 %x, 0
-; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 2147483647, i32 %x, !prof ![[MD1]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 %x, 2147483647
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 %x, i32 2147483647, !prof ![[MD3]]
; CHECK-NEXT: ret i32 [[SEL]]
;
%cmp = icmp slt i32 %x, 0
@@ -279,11 +273,10 @@ define i32 @umin2(i32 %x) {
ret i32 %sel
}
-; FIXME:
; The compare should change, but the metadata remains the same because the select operands are not swapped.
define i32 @umax1(i32 %x) {
; CHECK-LABEL: @umax1(
-; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 %x, 0
+; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i32 %x, 2147483647
; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 %x, i32 2147483647, !prof ![[MD1]]
; CHECK-NEXT: ret i32 [[SEL]]
;
@@ -292,12 +285,11 @@ define i32 @umax1(i32 %x) {
ret i32 %sel
}
-; FIXME:
; The compare should change, and the metadata is swapped because the select operands are swapped.
define i32 @umax2(i32 %x) {
; CHECK-LABEL: @umax2(
-; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 %x, -1
-; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 -2147483648, i32 %x, !prof ![[MD1]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i32 %x, -2147483648
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 %x, i32 -2147483648, !prof ![[MD3]]
; CHECK-NEXT: ret i32 [[SEL]]
;
%cmp = icmp sgt i32 %x, -1
More information about the llvm-commits
mailing list