[llvm-branch-commits] [llvm-branch] r293947 - Merging r293345:
Hans Wennborg via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Feb 2 14:19:34 PST 2017
Author: hans
Date: Thu Feb 2 16:19:34 2017
New Revision: 293947
URL: http://llvm.org/viewvc/llvm-project?rev=293947&view=rev
Log:
Merging r293345:
------------------------------------------------------------------------
r293345 | spatel | 2017-01-27 15:26:27 -0800 (Fri, 27 Jan 2017) | 19 lines
[InstCombine] move icmp transforms that might be recognized as min/max and inf-loop (PR31751)
This is a minimal patch to avoid the infinite loop in:
https://llvm.org/bugs/show_bug.cgi?id=31751
But the general problem is bigger: we're not canonicalizing all of the min/max forms reported
by value tracking's matchSelectPattern(), and we don't define min/max consistently. Some code
uses matchSelectPattern(), other code uses matchers like m_Umax, and others have their own
inline definitions which may be subtly different from any of the above.
The reason that the test cases in this patch need a cast op to trigger is because we don't
(yet) canonicalize all min/max forms based on matchSelectPattern() in
canonicalizeMinMaxWithConstant(), but we do make min/max+cast transforms based on
matchSelectPattern() in visitSelectInst().
The location of the icmp transforms that trigger the inf-loop seems arbitrary at best, so
I'm moving those behind the min/max fence in visitICmpInst() as the quick fix.
------------------------------------------------------------------------
Modified:
llvm/branches/release_40/ (props changed)
llvm/branches/release_40/lib/Transforms/InstCombine/InstCombineCompares.cpp
llvm/branches/release_40/test/Transforms/InstCombine/minmax-fold.ll
Propchange: llvm/branches/release_40/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Feb 2 16:19:34 2017
@@ -1,3 +1,3 @@
/llvm/branches/Apple/Pertwee:110850,110961
/llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,291858-291859,291863,291875,291909,291918,291966,291968,291979,292117,292133,292167,292169-292170,292242,292254-292255,292280,292323,292444,292467,292516,292583,292624-292625,292641,292651,292667,292711-292713,292758,293021,293025,293230,293259,293291,293293,293309,293417,293522,293542,293629,293635,293658,293673,293727,293730
+/llvm/trunk:155241,291858-291859,291863,291875,291909,291918,291966,291968,291979,292117,292133,292167,292169-292170,292242,292254-292255,292280,292323,292444,292467,292516,292583,292624-292625,292641,292651,292667,292711-292713,292758,293021,293025,293230,293259,293291,293293,293309,293345,293417,293522,293542,293629,293635,293658,293673,293727,293730
Modified: llvm/branches/release_40/lib/Transforms/InstCombine/InstCombineCompares.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_40/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=293947&r1=293946&r2=293947&view=diff
==============================================================================
--- llvm/branches/release_40/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
+++ llvm/branches/release_40/lib/Transforms/InstCombine/InstCombineCompares.cpp Thu Feb 2 16:19:34 2017
@@ -4039,11 +4039,6 @@ Instruction *InstCombiner::foldICmpUsing
Constant *CMinus1 = ConstantInt::get(Op0->getType(), *CmpC - 1);
return new ICmpInst(ICmpInst::ICMP_EQ, Op0, CMinus1);
}
- // (x <u 2147483648) -> (x >s -1) -> true if sign bit clear
- if (CmpC->isMinSignedValue()) {
- Constant *AllOnes = Constant::getAllOnesValue(Op0->getType());
- return new ICmpInst(ICmpInst::ICMP_SGT, Op0, AllOnes);
- }
}
break;
}
@@ -4063,11 +4058,6 @@ Instruction *InstCombiner::foldICmpUsing
if (*CmpC == Op0Max - 1)
return new ICmpInst(ICmpInst::ICMP_EQ, Op0,
ConstantInt::get(Op1->getType(), *CmpC + 1));
-
- // (x >u 2147483647) -> (x <s 0) -> true if sign bit set
- if (CmpC->isMaxSignedValue())
- return new ICmpInst(ICmpInst::ICMP_SLT, Op0,
- Constant::getNullValue(Op0->getType()));
}
break;
}
@@ -4299,6 +4289,27 @@ Instruction *InstCombiner::visitICmpInst
(SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1))
return nullptr;
+ // FIXME: We only do this after checking for min/max to prevent infinite
+ // looping caused by a reverse canonicalization of these patterns for min/max.
+ // FIXME: The organization of folds is a mess. These would naturally go into
+ // canonicalizeCmpWithConstant(), but we can't move all of the above folds
+ // down here after the min/max restriction.
+ ICmpInst::Predicate Pred = I.getPredicate();
+ const APInt *C;
+ if (match(Op1, m_APInt(C))) {
+ // For i32: x >u 2147483647 -> x <s 0 -> true if sign bit set
+ if (Pred == ICmpInst::ICMP_UGT && C->isMaxSignedValue()) {
+ Constant *Zero = Constant::getNullValue(Op0->getType());
+ return new ICmpInst(ICmpInst::ICMP_SLT, Op0, Zero);
+ }
+
+ // For i32: x <u 2147483648 -> x >s -1 -> true if sign bit clear
+ if (Pred == ICmpInst::ICMP_ULT && C->isMinSignedValue()) {
+ Constant *AllOnes = Constant::getAllOnesValue(Op0->getType());
+ return new ICmpInst(ICmpInst::ICMP_SGT, Op0, AllOnes);
+ }
+ }
+
if (Instruction *Res = foldICmpInstWithConstant(I))
return Res;
Modified: llvm/branches/release_40/test/Transforms/InstCombine/minmax-fold.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_40/test/Transforms/InstCombine/minmax-fold.ll?rev=293947&r1=293946&r2=293947&view=diff
==============================================================================
--- llvm/branches/release_40/test/Transforms/InstCombine/minmax-fold.ll (original)
+++ llvm/branches/release_40/test/Transforms/InstCombine/minmax-fold.ll Thu Feb 2 16:19:34 2017
@@ -339,3 +339,84 @@ define i32 @test75(i32 %x) {
ret i32 %retval
}
+; The next 3 min tests should canonicalize to the same form...and not infinite loop.
+
+define double @PR31751_umin1(i32 %x) {
+; CHECK-LABEL: @PR31751_umin1(
+; CHECK-NEXT: [[TMP1:%.*]] = icmp ult i32 %x, 2147483647
+; CHECK-NEXT: [[CONV1:%.*]] = select i1 [[TMP1]], i32 %x, i32 2147483647
+; CHECK-NEXT: [[TMP2:%.*]] = sitofp i32 [[CONV1]] to double
+; CHECK-NEXT: ret double [[TMP2]]
+;
+ %cmp = icmp slt i32 %x, 0
+ %sel = select i1 %cmp, i32 2147483647, i32 %x
+ %conv = sitofp i32 %sel to double
+ ret double %conv
+}
+
+define double @PR31751_umin2(i32 %x) {
+; CHECK-LABEL: @PR31751_umin2(
+; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 %x, 2147483647
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 %x, i32 2147483647
+; CHECK-NEXT: [[CONV:%.*]] = sitofp i32 [[SEL]] to double
+; CHECK-NEXT: ret double [[CONV]]
+;
+ %cmp = icmp ult i32 %x, 2147483647
+ %sel = select i1 %cmp, i32 %x, i32 2147483647
+ %conv = sitofp i32 %sel to double
+ ret double %conv
+}
+
+define double @PR31751_umin3(i32 %x) {
+; CHECK-LABEL: @PR31751_umin3(
+; CHECK-NEXT: [[TMP1:%.*]] = icmp ult i32 %x, 2147483647
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[TMP1]], i32 %x, i32 2147483647
+; CHECK-NEXT: [[CONV:%.*]] = sitofp i32 [[SEL]] to double
+; CHECK-NEXT: ret double [[CONV]]
+;
+ %cmp = icmp ugt i32 %x, 2147483647
+ %sel = select i1 %cmp, i32 2147483647, i32 %x
+ %conv = sitofp i32 %sel to double
+ ret double %conv
+}
+
+; The next 3 max tests should canonicalize to the same form...and not infinite loop.
+
+define double @PR31751_umax1(i32 %x) {
+; CHECK-LABEL: @PR31751_umax1(
+; CHECK-NEXT: [[TMP1:%.*]] = icmp ugt i32 %x, -2147483648
+; CHECK-NEXT: [[CONV1:%.*]] = select i1 [[TMP1]], i32 %x, i32 -2147483648
+; CHECK-NEXT: [[TMP2:%.*]] = sitofp i32 [[CONV1]] to double
+; CHECK-NEXT: ret double [[TMP2]]
+;
+ %cmp = icmp sgt i32 %x, -1
+ %sel = select i1 %cmp, i32 2147483648, i32 %x
+ %conv = sitofp i32 %sel to double
+ ret double %conv
+}
+
+define double @PR31751_umax2(i32 %x) {
+; CHECK-LABEL: @PR31751_umax2(
+; CHECK-NEXT: [[CMP:%.*]] = icmp ugt i32 %x, -2147483648
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 %x, i32 -2147483648
+; CHECK-NEXT: [[CONV:%.*]] = sitofp i32 [[SEL]] to double
+; CHECK-NEXT: ret double [[CONV]]
+;
+ %cmp = icmp ugt i32 %x, 2147483648
+ %sel = select i1 %cmp, i32 %x, i32 2147483648
+ %conv = sitofp i32 %sel to double
+ ret double %conv
+}
+
+define double @PR31751_umax3(i32 %x) {
+; CHECK-LABEL: @PR31751_umax3(
+; CHECK-NEXT: [[TMP1:%.*]] = icmp ugt i32 %x, -2147483648
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[TMP1]], i32 %x, i32 -2147483648
+; CHECK-NEXT: [[CONV:%.*]] = sitofp i32 [[SEL]] to double
+; CHECK-NEXT: ret double [[CONV]]
+;
+ %cmp = icmp ult i32 %x, 2147483648
+ %sel = select i1 %cmp, i32 2147483648, i32 %x
+ %conv = sitofp i32 %sel to double
+ ret double %conv
+}
More information about the llvm-branch-commits
mailing list