[PATCH] D16915: Fix for bug 26465
Taewook Oh via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 4 23:08:44 PST 2016
twoh created this revision.
twoh added reviewers: hfinkel, majnemer, spatel.
twoh added a subscriber: llvm-commits.
twoh set the repository for this revision to rL LLVM.
For some cases, InstCombine replaces the sequence of xor/sub instruction followed by cmp instruction into a single cmp instruction. However, this replacement may result suboptimal result especially when the xor/sub has more than one use, as discussed in bug 26465 (https://llvm.org/bugs/show_bug.cgi?id=26465). This patch make the replacement happen only when xor/sub has only one use. Below are test codes for each case that addressed by this patch. For all cases below, instcombine pass does not change the original code at all when the fix is applied.
*** Case 1: ((xor A, ConstB) == ConstC) -> (A == (ConstB xor ConstC))
# test case
define zeroext i1 @foo(i32 %lhs, i32 %rhs) {
%xor = xor i32 %lhs, 5
%cmp1 = icmp eq i32 %xor, 10
%cmp2 = icmp eq i32 %xor, %rhs
%sel = or i1 %cmp1, %cmp2
ret i1 %sel
}
# Output after instcombine without fix
define zeroext i1 @foo(i32 %lhs, i32 %rhs) {
%xor = xor i32 %lhs, 5
%cmp1 = icmp eq i32 %lhs, 15
%cmp2 = icmp eq i32 %xor, %rhs
%sel = or i1 %cmp1, %cmp2
ret i1 %sel
}
*** Case 2: ((xor A, B) == 0) with (A == B)
# test case
define zeroext i1 @foo(i32 %lhs, i32 %rhs) {
%xor = xor i32 %lhs, %rhs
%cmp1 = icmp eq i32 %xor, 0
%cmp2 = icmp eq i32 %xor, 32
%sel = xor i1 %cmp1, %cmp2
ret i1 %sel
}
# Output after instcombine without fix
define zeroext i1 @foo(i32 %lhs, i32 %rhs) {
%xor = xor i32 %lhs, %rhs
%cmp1 = icmp eq i32 %lhs, %rhs
%cmp2 = icmp eq i32 %xor, 32
%sel = xor i1 %cmp1, %cmp2
ret i1 %sel
}
*** Case 3: ((sub A, B) == 0) -> (A == B)
# test case
define zeroext i1 @foo(i32 %lhs, i32 %rhs) {
%sub = sub nsw i32 %lhs, %rhs
%cmp1 = icmp eq i32 %sub, 0
%cmp2 = icmp eq i32 %sub, 31
%sel = or i1 %cmp1, %cmp2 ret i1 %sel
}
# Output after instcombine without fix
define zeroext i1 @foo(i32 %lhs, i32 %rhs) {
%sub = sub nsw i32 %lhs, %rhs
%cmp1 = icmp eq i32 %lhs, %rhs
%cmp2 = icmp eq i32 %sub, 31
%sel = or i1 %cmp1, %cmp2
ret i1 %sel
}
Repository:
rL LLVM
http://reviews.llvm.org/D16915
Files:
lib/Transforms/InstCombine/InstCombineCompares.cpp
Index: lib/Transforms/InstCombine/InstCombineCompares.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -1709,7 +1709,7 @@
cast<ConstantInt>(ConstantExpr::getShl(AndCst, ShAmt));
ConstantInt *ShiftedRHSCst =
cast<ConstantInt>(ConstantExpr::getShl(RHS, ShAmt));
-
+
if (!ShiftedAndCst->isNegative() && !ShiftedRHSCst->isNegative())
CanFold = true;
}
@@ -2214,25 +2214,28 @@
case Instruction::Xor:
// For the xor case, we can xor two constants together, eliminating
// the explicit xor.
- if (Constant *BOC = dyn_cast<Constant>(BO->getOperand(1))) {
- return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
- ConstantExpr::getXor(RHS, BOC));
- } else if (RHSV == 0) {
- // Replace ((xor A, B) != 0) with (A != B)
- return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
- BO->getOperand(1));
+ if (BO->hasOneUse()) {
+ if (Constant *BOC = dyn_cast<Constant>(BO->getOperand(1))) {
+ return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
+ ConstantExpr::getXor(RHS, BOC));
+ } else if (RHSV == 0) {
+ // Replace ((xor A, B) != 0) with (A != B)
+ return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
+ BO->getOperand(1));
+ }
}
break;
case Instruction::Sub:
// Replace ((sub A, B) != C) with (B != A-C) if A & C are constants.
- if (ConstantInt *BOp0C = dyn_cast<ConstantInt>(BO->getOperand(0))) {
- if (BO->hasOneUse())
+ if (BO->hasOneUse()) {
+ if (ConstantInt *BOp0C = dyn_cast<ConstantInt>(BO->getOperand(0))) {
return new ICmpInst(ICI.getPredicate(), BO->getOperand(1),
- ConstantExpr::getSub(BOp0C, RHS));
- } else if (RHSV == 0) {
- // Replace ((sub A, B) != 0) with (A != B)
- return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
- BO->getOperand(1));
+ ConstantExpr::getSub(BOp0C, RHS));
+ } else if (RHSV == 0) {
+ // Replace ((sub A, B) != 0) with (A != B)
+ return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
+ BO->getOperand(1));
+ }
}
break;
case Instruction::Or:
@@ -4197,21 +4200,21 @@
// This would allow us to handle (fptosi (x >>s 62) to float) if x is i64 f.e.
unsigned InputSize = IntTy->getScalarSizeInBits();
- // Following test does NOT adjust InputSize downwards for signed inputs,
- // because the most negative value still requires all the mantissa bits
+ // Following test does NOT adjust InputSize downwards for signed inputs,
+ // because the most negative value still requires all the mantissa bits
// to distinguish it from one less than that value.
if ((int)InputSize > MantissaWidth) {
// Conversion would lose accuracy. Check if loss can impact comparison.
int Exp = ilogb(RHS);
if (Exp == APFloat::IEK_Inf) {
int MaxExponent = ilogb(APFloat::getLargest(RHS.getSemantics()));
- if (MaxExponent < (int)InputSize - !LHSUnsigned)
+ if (MaxExponent < (int)InputSize - !LHSUnsigned)
// Conversion could create infinity.
return nullptr;
} else {
- // Note that if RHS is zero or NaN, then Exp is negative
+ // Note that if RHS is zero or NaN, then Exp is negative
// and first condition is trivially false.
- if (MantissaWidth <= Exp && Exp <= (int)InputSize - !LHSUnsigned)
+ if (MantissaWidth <= Exp && Exp <= (int)InputSize - !LHSUnsigned)
// Conversion could affect comparison.
return nullptr;
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16915.46994.patch
Type: text/x-patch
Size: 3995 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160205/d32bf429/attachment.bin>
More information about the llvm-commits
mailing list