<div dir="ltr">Sanjay,<div><br></div><div>I think this change is breaking both ppc64 sanitizer build bots.</div><div><br></div><div>First breakage:</div><div><br></div><div><a href="http://lab.llvm.org:8011/builders/sanitizer-ppc64be-linux/builds/2016" class="cremed">http://lab.llvm.org:8011/builders/sanitizer-ppc64be-linux/builds/2016</a></div><div><br></div><div>Not sure why is it PPC specific.</div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, May 16, 2016 at 6:04 PM Sanjay Patel via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: spatel<br>
Date: Mon May 16 19:57:57 2016<br>
New Revision: 269728<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=269728&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=269728&view=rev</a><br>
Log:<br>
[InstCombine] check vector elements before trying to transform LE/GE vector icmp (PR27756)<br>
<br>
Fix a bug introduced with rL269426 :<br>
[InstCombine] canonicalize* LE/GE vector integer comparisons to LT/GT (PR26701, PR26819)<br>
<br>
We were assuming that a ConstantDataVector / ConstantVector / ConstantAggregateZero operand of<br>
an ICMP was composed of ConstantInt elements, but it might have ConstantExpr or UndefValue<br>
elements. Handle those appropriately.<br>
<br>
Also, refactor this function to join the scalar and vector paths and eliminate the switches.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D20289" rel="noreferrer" target="_blank">http://reviews.llvm.org/D20289</a><br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
llvm/trunk/test/Transforms/InstCombine/icmp-vec.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=269728&r1=269727&r2=269728&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=269728&r1=269727&r2=269728&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Mon May 16 19:57:57 2016<br>
@@ -3102,90 +3102,54 @@ bool InstCombiner::replacedSelectWithOpe<br>
/// If we have an icmp le or icmp ge instruction with a constant operand, turn<br>
/// it into the appropriate icmp lt or icmp gt instruction. This transform<br>
/// allows them to be folded in visitICmpInst.<br>
-static ICmpInst *canonicalizeCmpWithConstant(ICmpInst &I,<br>
- InstCombiner::BuilderTy &Builder) {<br>
+static ICmpInst *canonicalizeCmpWithConstant(ICmpInst &I) {<br>
+ ICmpInst::Predicate Pred = I.getPredicate();<br>
+ if (Pred != ICmpInst::ICMP_SLE && Pred != ICmpInst::ICMP_SGE &&<br>
+ Pred != ICmpInst::ICMP_ULE && Pred != ICmpInst::ICMP_UGE)<br>
+ return nullptr;<br>
+<br>
Value *Op0 = I.getOperand(0);<br>
Value *Op1 = I.getOperand(1);<br>
-<br>
- if (auto *Op1C = dyn_cast<ConstantInt>(Op1)) {<br>
- // For scalars, SimplifyICmpInst has already handled the edge cases for us,<br>
- // so we just assert on them.<br>
- APInt Op1Val = Op1C->getValue();<br>
- switch (I.getPredicate()) {<br>
- case ICmpInst::ICMP_ULE:<br>
- assert(!Op1C->isMaxValue(false)); // A <=u MAX -> TRUE<br>
- return new ICmpInst(ICmpInst::ICMP_ULT, Op0, Builder.getInt(Op1Val + 1));<br>
- case ICmpInst::ICMP_SLE:<br>
- assert(!Op1C->isMaxValue(true)); // A <=s MAX -> TRUE<br>
- return new ICmpInst(ICmpInst::ICMP_SLT, Op0, Builder.getInt(Op1Val + 1));<br>
- case ICmpInst::ICMP_UGE:<br>
- assert(!Op1C->isMinValue(false)); // A >=u MIN -> TRUE<br>
- return new ICmpInst(ICmpInst::ICMP_UGT, Op0, Builder.getInt(Op1Val - 1));<br>
- case ICmpInst::ICMP_SGE:<br>
- assert(!Op1C->isMinValue(true)); // A >=s MIN -> TRUE<br>
- return new ICmpInst(ICmpInst::ICMP_SGT, Op0, Builder.getInt(Op1Val - 1));<br>
- default:<br>
- return nullptr;<br>
- }<br>
- }<br>
-<br>
- // The usual vector types are ConstantDataVector. Exotic vector types are<br>
- // ConstantVector. Zeros are special. They all derive from Constant.<br>
- if (isa<ConstantDataVector>(Op1) || isa<ConstantVector>(Op1) ||<br>
- isa<ConstantAggregateZero>(Op1)) {<br>
- Constant *Op1C = cast<Constant>(Op1);<br>
- Type *Op1Type = Op1->getType();<br>
- unsigned NumElts = Op1Type->getVectorNumElements();<br>
-<br>
- // Set the new comparison predicate and splat a vector of 1 or -1 to<br>
- // increment or decrement the vector constants. But first, check that no<br>
- // elements of the constant vector would overflow/underflow when we<br>
- // increment/decrement the constants.<br>
- //<br>
+ auto *Op1C = dyn_cast<Constant>(Op1);<br>
+ if (!Op1C)<br>
+ return nullptr;<br>
+<br>
+ // Check if the constant operand can be safely incremented/decremented without<br>
+ // overflowing/underflowing. For scalars, SimplifyICmpInst has already handled<br>
+ // the edge cases for us, so we just assert on them. For vectors, we must<br>
+ // handle the edge cases.<br>
+ Type *Op1Type = Op1->getType();<br>
+ bool IsSigned = I.isSigned();<br>
+ bool IsLE = (Pred == ICmpInst::ICMP_SLE || Pred == ICmpInst::ICMP_ULE);<br>
+ if (auto *CI = dyn_cast<ConstantInt>(Op1C)) {<br>
+ // A <= MAX -> TRUE ; A >= MIN -> TRUE<br>
+ assert(IsLE ? !CI->isMaxValue(IsSigned) : !CI->isMinValue(IsSigned));<br>
+ } else if (Op1Type->isVectorTy()) {<br>
// TODO? If the edge cases for vectors were guaranteed to be handled as they<br>
- // are for scalar, we could remove the min/max checks here. However, to do<br>
- // that, we would have to use insertelement/shufflevector to replace edge<br>
- // values.<br>
-<br>
- CmpInst::Predicate NewPred;<br>
- Constant *OneOrNegOne = nullptr;<br>
- switch (I.getPredicate()) {<br>
- case ICmpInst::ICMP_ULE:<br>
- for (unsigned i = 0; i != NumElts; ++i)<br>
- if (cast<ConstantInt>(Op1C->getAggregateElement(i))->isMaxValue(false))<br>
- return nullptr;<br>
- NewPred = ICmpInst::ICMP_ULT;<br>
- OneOrNegOne = ConstantInt::get(Op1Type, 1);<br>
- break;<br>
- case ICmpInst::ICMP_SLE:<br>
- for (unsigned i = 0; i != NumElts; ++i)<br>
- if (cast<ConstantInt>(Op1C->getAggregateElement(i))->isMaxValue(true))<br>
- return nullptr;<br>
- NewPred = ICmpInst::ICMP_SLT;<br>
- OneOrNegOne = ConstantInt::get(Op1Type, 1);<br>
- break;<br>
- case ICmpInst::ICMP_UGE:<br>
- for (unsigned i = 0; i != NumElts; ++i)<br>
- if (cast<ConstantInt>(Op1C->getAggregateElement(i))->isMinValue(false))<br>
- return nullptr;<br>
- NewPred = ICmpInst::ICMP_UGT;<br>
- OneOrNegOne = ConstantInt::get(Op1Type, -1);<br>
- break;<br>
- case ICmpInst::ICMP_SGE:<br>
- for (unsigned i = 0; i != NumElts; ++i)<br>
- if (cast<ConstantInt>(Op1C->getAggregateElement(i))->isMinValue(true))<br>
- return nullptr;<br>
- NewPred = ICmpInst::ICMP_SGT;<br>
- OneOrNegOne = ConstantInt::get(Op1Type, -1);<br>
- break;<br>
- default:<br>
- return nullptr;<br>
- }<br>
-<br>
- return new ICmpInst(NewPred, Op0, ConstantExpr::getAdd(Op1C, OneOrNegOne));<br>
+ // are for scalar, we could remove the min/max checks. However, to do that,<br>
+ // we would have to use insertelement/shufflevector to replace edge values.<br>
+ unsigned NumElts = Op1Type->getVectorNumElements();<br>
+ for (unsigned i = 0; i != NumElts; ++i) {<br>
+ Constant *Elt = Op1C->getAggregateElement(i);<br>
+ if (isa<UndefValue>(Elt))<br>
+ continue;<br>
+ // Bail out if we can't determine if this constant is min/max or if we<br>
+ // know that this constant is min/max.<br>
+ auto *CI = dyn_cast<ConstantInt>(Elt);<br>
+ if (!CI || (IsLE ? CI->isMaxValue(IsSigned) : CI->isMinValue(IsSigned)))<br>
+ return nullptr;<br>
+ }<br>
+ } else {<br>
+ // ConstantExpr?<br>
+ return nullptr;<br>
}<br>
<br>
- return nullptr;<br>
+ // Increment or decrement the constant and set the new comparison predicate:<br>
+ // ULE -> ULT ; UGE -> UGT ; SLE -> SLT ; SGE -> SGT<br>
+ Constant *OneOrNegOne = ConstantInt::get(Op1Type, IsLE ? 1 : -1);<br>
+ CmpInst::Predicate NewPred = IsLE ? ICmpInst::ICMP_ULT: ICmpInst::ICMP_UGT;<br>
+ NewPred = IsSigned ? ICmpInst::getSignedPredicate(NewPred) : NewPred;<br>
+ return new ICmpInst(NewPred, Op0, ConstantExpr::getAdd(Op1C, OneOrNegOne));<br>
}<br>
<br>
Instruction *InstCombiner::visitICmpInst(ICmpInst &I) {<br>
@@ -3271,7 +3235,7 @@ Instruction *InstCombiner::visitICmpInst<br>
}<br>
}<br>
<br>
- if (ICmpInst *NewICmp = canonicalizeCmpWithConstant(I, *Builder))<br>
+ if (ICmpInst *NewICmp = canonicalizeCmpWithConstant(I))<br>
return NewICmp;<br>
<br>
unsigned BitWidth = 0;<br>
<br>
Modified: llvm/trunk/test/Transforms/InstCombine/icmp-vec.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/icmp-vec.ll?rev=269728&r1=269727&r2=269728&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/icmp-vec.ll?rev=269728&r1=269727&r2=269728&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/InstCombine/icmp-vec.ll (original)<br>
+++ llvm/trunk/test/Transforms/InstCombine/icmp-vec.ll Mon May 16 19:57:57 2016<br>
@@ -159,3 +159,25 @@ define <2 x i1> @ule_max(<2 x i3> %x) {<br>
ret <2 x i1> %cmp<br>
}<br>
<br>
+; If we can't determine if a constant element is min/max (eg, it's a ConstantExpr), do nothing.<br>
+<br>
+define <2 x i1> @PR27756_1(<2 x i8> %a) {<br>
+; CHECK-LABEL: @PR27756_1(<br>
+; CHECK-NEXT: [[CMP:%.*]] = icmp sle <2 x i8> %a, <i8 bitcast (<2 x i4> <i4 1, i4 2> to i8), i8 0><br>
+; CHECK-NEXT: ret <2 x i1> [[CMP]]<br>
+;<br>
+ %cmp = icmp sle <2 x i8> %a, <i8 bitcast (<2 x i4> <i4 1, i4 2> to i8), i8 0><br>
+ ret <2 x i1> %cmp<br>
+}<br>
+<br>
+; Undef elements don't prevent the transform of the comparison.<br>
+<br>
+define <2 x i1> @PR27756_2(<2 x i8> %a) {<br>
+; CHECK-LABEL: @PR27756_2(<br>
+; CHECK-NEXT: [[CMP:%.*]] = icmp slt <2 x i8> %a, <i8 undef, i8 1><br>
+; CHECK-NEXT: ret <2 x i1> [[CMP]]<br>
+;<br>
+ %cmp = icmp sle <2 x i8> %a, <i8 undef, i8 0><br>
+ ret <2 x i1> %cmp<br>
+}<br>
+<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><div dir="ltr">-- <br></div>Mike<br>Sent from phone