RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2025-01-31 12:41:49 +01:00
Code Issues Projects Releases Wiki Activity

[InstCombine] Fix wrong folding of constant comparison involving ahsr and negative quantities (PR20945).

Browse Source 
Example:
define i1 @foo(i32 %a) {
  %shr = ashr i32 -9, %a
  %cmp = icmp ne i32 %shr, -5
  ret i1 %cmp
}

Before this fix, the instruction combiner wrongly thought that %shr
could have never been equal to -5. Therefore, %cmp was always folded to 'true'.
However, when %a is equal to 1, then %cmp evaluates to 'false'. Therefore,
in this example, it is not valid to fold %cmp to 'true'.
The problem was only affecting the case where the comparison was between
negative quantities where one of the quantities was obtained from arithmetic
shift of a negative constant.

This patch fixes the problem with the wrong folding (fixes PR20945).
With this patch, the 'icmp' from the example is now simplified to a
comparison between %a and 1. This still allows us to get rid of the arithmetic
shift (%shr).

llvm-svn: 217950
This commit is contained in:
Andrea Di Biagio 2014-09-17 11:32:31 +00:00
parent c2c9440ff0
commit 99dc03a95d
2 changed files with 28 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
lib/Transforms/InstCombine/InstCombineCompares.cpp
View File

@ -1085,29 +1085,33 @@ Instruction *InstCombiner::FoldICmpCstShrCst(ICmpInst &I, Value *Op, Value *A,
return getICmp(I.ICMP_EQ, A, ConstantInt::getNullValue(A->getType()));
}
bool IsNegative = false;
if (IsAShr) {
if (AP1.isNegative() != AP2.isNegative()) {
// Arithmetic shift will never change the sign.
return getConstant(false);
}
// Both the constants are negative, take their positive to calculate
// log.
// Both the constants are negative, take their positive to calculate log.
if (AP1.isNegative()) {
AP1 = -AP1;
AP2 = -AP2;
if (AP1.slt(AP2))
// Right-shifting won't increase the magnitude.
return getConstant(false);
IsNegative = true;
}
}
if (AP1.ugt(AP2)) {
if (!IsNegative && AP1.ugt(AP2))
// Right-shifting will not increase the value.
return getConstant(false);
}
// Get the distance between the highest bit that's set.
int Shift = AP2.logBase2() - AP1.logBase2();
int Shift;
if (IsNegative)
Shift = (-AP2).logBase2() - (-AP1).logBase2();
else
Shift = AP2.logBase2() - AP1.logBase2();
// Use lshr here, since we've canonicalized to +ve numbers.
if (AP1 == AP2.lshr(Shift))
if (IsAShr ? AP1 == AP2.ashr(Shift) : AP1 == AP2.lshr(Shift))
return getICmp(I.ICMP_EQ, A, ConstantInt::get(A->getType(), Shift));
// Shifting const2 will never be equal to const1.

15
test/Transforms/InstCombine/icmp-shr.ll
View File

@ -675,3 +675,18 @@ define i1 @nonexact_ashr_ne_noexactlog(i8 %a) {
%cmp = icmp ne i8 %shr, -30
ret i1 %cmp
}
; Don't try to fold the entire body of function @PR20945 into a
; single `ret i1 true` statement.
; If %B is equal to 1, then this function would return false.
; As a consequence, the instruction combiner is not allowed to fold %cmp
; to 'true'. Instead, it should replace %cmp with a simpler comparison
; between %B and 1.
; CHECK-LABEL: @PR20945(
; CHECK: icmp ne i32 %B, 1
define i1 @PR20945(i32 %B) {
%shr = ashr i32 -9, %B
%cmp = icmp ne i32 %shr, -5
ret i1 %cmp
}