[llvm-commits] [llvm] r151467 - in /llvm/trunk: lib/Analysis/InstructionSimplify.cpp test/Transforms/InstCombine/icmp.ll test/Transforms/InstSimplify/compare.ll
Nick Lewycky
nicholas at mxc.ca
Tue Feb 28 23:49:47 PST 2012
Eli Friedman wrote:
> On Sat, Feb 25, 2012 at 6:09 PM, Nick Lewycky<nicholas at mxc.ca> wrote:
>> Author: nicholas
>> Date: Sat Feb 25 20:09:49 2012
>> New Revision: 151467
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=151467&view=rev
>> Log:
>> Reinstate the optimization from r151449 with a fix to not turn 'gep %x' into
>> 'gep null' when the icmp predicate is unsigned (or is signed without inbounds).
>>
>> Modified:
>> llvm/trunk/lib/Analysis/InstructionSimplify.cpp
>> llvm/trunk/test/Transforms/InstCombine/icmp.ll
>> llvm/trunk/test/Transforms/InstSimplify/compare.ll
>
> This change is causing an execution failure in the gcc testsuite on
> gcc.c-torture/execute/frame-address.c . Please take a look.
int check_fa_work (const char *c, const char *f)
{
const char d = 0;
if (c >= &d)
return c >= f && f >= &d;
else
return c <= f && f <= &d;
}
With this patch we decide that the incoming arguments can't legitimately
be compared with the local variable, taking advantage of the undefined
behaviour. This function is optimized into "ret i32 0".
This test is trying to use multiple functions to hide from the fact that
the optimizer would've done that if they wrote it in a single function.
Notably:
target datalayout =
"e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
define i1 @test() {
%A = alloca i8
%B = alloca i8
%cmp = icmp slt i8* %A, %B
ret i1 %cmp
}
folds into 'ret i1 true' without my patch. (Notably it still folds into
'ret i1 true' even if you swap the slt for sgt! And that's correct.)
Nick
>
> -Eli
>
>> Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=151467&r1=151466&r2=151467&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
>> +++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Sat Feb 25 20:09:49 2012
>> @@ -21,6 +21,7 @@
>> #include "llvm/Operator.h"
>> #include "llvm/ADT/Statistic.h"
>> #include "llvm/Analysis/InstructionSimplify.h"
>> +#include "llvm/Analysis/AliasAnalysis.h"
>> #include "llvm/Analysis/ConstantFolding.h"
>> #include "llvm/Analysis/Dominators.h"
>> #include "llvm/Analysis/ValueTracking.h"
>> @@ -1609,26 +1610,43 @@
>> }
>> }
>>
>> - // icmp<alloca*>,<global/alloca*/null> - Different stack variables have
>> - // different addresses, and what's more the address of a stack variable is
>> - // never null or equal to the address of a global. Note that generalizing
>> - // to the case where LHS is a global variable address or null is pointless,
>> - // since if both LHS and RHS are constants then we already constant folded
>> - // the compare, and if only one of them is then we moved it to RHS already.
>> + // icmp<object*>,<object*/null> - Different identified objects have
>> + // different addresses (unless null), and what's more the address of an
>> + // identified local is never equal to another argument (again, barring null).
>> + // Note that generalizing to the case where LHS is a global variable address
>> + // or null is pointless, since if both LHS and RHS are constants then we
>> + // already constant folded the compare, and if only one of them is then we
>> + // moved it to RHS already.
>> Value *LHSPtr = LHS->stripPointerCasts();
>> Value *RHSPtr = RHS->stripPointerCasts();
>> if (LHSPtr == RHSPtr)
>> return ConstantInt::get(ITy, CmpInst::isTrueWhenEqual(Pred));
>> -
>> +
>> // Be more aggressive about stripping pointer adjustments when checking a
>> // comparison of an alloca address to another object. We can rip off all
>> // inbounds GEP operations, even if they are variable.
>> LHSPtr = stripPointerAdjustments(LHSPtr);
>> - if (isa<AllocaInst>(LHSPtr)) {
>> + if (llvm::isIdentifiedObject(LHSPtr)) {
>> + RHSPtr = stripPointerAdjustments(RHSPtr);
>> + if (llvm::isKnownNonNull(LHSPtr) || llvm::isKnownNonNull(RHSPtr)) {
>> + // If both sides are different identified objects, they aren't equal
>> + // unless they're null.
>> + if (LHSPtr != RHSPtr&& llvm::isIdentifiedObject(RHSPtr))
>> + return ConstantInt::get(ITy, CmpInst::isFalseWhenEqual(Pred));
>> +
>> + // A local identified object (alloca or noalias call) can't equal any
>> + // incoming argument, unless they're both null.
>> + if (isa<Instruction>(LHSPtr)&& isa<Argument>(RHSPtr))
>> + return ConstantInt::get(ITy, CmpInst::isFalseWhenEqual(Pred));
>> + }
>> +
>> + // Assume that the constant null is on the right.
>> + if (llvm::isKnownNonNull(LHSPtr)&& isa<ConstantPointerNull>(RHSPtr))
>> + return ConstantInt::get(ITy, CmpInst::isFalseWhenEqual(Pred));
>> + } else if (isa<Argument>(LHSPtr)) {
>> RHSPtr = stripPointerAdjustments(RHSPtr);
>> - if (LHSPtr != RHSPtr&&
>> - (isa<GlobalValue>(RHSPtr) || isa<AllocaInst>(RHSPtr) ||
>> - isa<ConstantPointerNull>(RHSPtr)))
>> + // An alloca can't be equal to an argument.
>> + if (isa<AllocaInst>(RHSPtr))
>> return ConstantInt::get(ITy, CmpInst::isFalseWhenEqual(Pred));
>> }
>>
>> @@ -2240,6 +2258,28 @@
>> return getFalse(ITy);
>> }
>>
>> + // Simplify comparisons of GEPs.
>> + if (GetElementPtrInst *GLHS = dyn_cast<GetElementPtrInst>(LHS)) {
>> + if (GEPOperator *GRHS = dyn_cast<GEPOperator>(RHS)) {
>> + if (GLHS->getPointerOperand() == GRHS->getPointerOperand()&&
>> + GLHS->hasAllConstantIndices()&& GRHS->hasAllConstantIndices()&&
>> + (ICmpInst::isEquality(Pred) ||
>> + (GLHS->isInBounds()&& GRHS->isInBounds()&&
>> + Pred == ICmpInst::getSignedPredicate(Pred)))) {
>> + // The bases are equal and the indices are constant. Build a constant
>> + // expression GEP with the same indices and a null base pointer to see
>> + // what constant folding can make out of it.
>> + Constant *Null = Constant::getNullValue(GLHS->getPointerOperandType());
>> + SmallVector<Value *, 4> IndicesLHS(GLHS->idx_begin(), GLHS->idx_end());
>> + Constant *NewLHS = ConstantExpr::getGetElementPtr(Null, IndicesLHS);
>> +
>> + SmallVector<Value *, 4> IndicesRHS(GRHS->idx_begin(), GRHS->idx_end());
>> + Constant *NewRHS = ConstantExpr::getGetElementPtr(Null, IndicesRHS);
>> + return ConstantExpr::getICmp(Pred, NewLHS, NewRHS);
>> + }
>> + }
>> + }
>> +
>> // If the comparison is with the result of a select instruction, check whether
>> // comparing with either branch of the select always yields the same value.
>> if (isa<SelectInst>(LHS) || isa<SelectInst>(RHS))
>>
>> Modified: llvm/trunk/test/Transforms/InstCombine/icmp.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/icmp.ll?rev=151467&r1=151466&r2=151467&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/icmp.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/icmp.ll Sat Feb 25 20:09:49 2012
>> @@ -634,8 +634,6 @@
>> %arrayidx2 = getelementptr inbounds i8* %a, i64 10
>> %cmp = icmp slt i8* %arrayidx1, %arrayidx2
>> ret i1 %cmp
>> -; Don't turn a signed cmp of GEPs into an index compare.
>> ; CHECK: @test62
>> -; CHECK: %cmp = icmp slt i8* %arrayidx1, %arrayidx2
>> -; CHECK-NEXT: ret i1 %cmp
>> +; CHECK-NEXT: ret i1 true
>> }
>>
>> Modified: llvm/trunk/test/Transforms/InstSimplify/compare.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/compare.ll?rev=151467&r1=151466&r2=151467&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstSimplify/compare.ll (original)
>> +++ llvm/trunk/test/Transforms/InstSimplify/compare.ll Sat Feb 25 20:09:49 2012
>> @@ -40,6 +40,69 @@
>> ; CHECK-NEXT: ret i1 true
>> }
>>
>> +; PR11238
>> +%gept = type { i32, i32 }
>> + at gepy = global %gept zeroinitializer, align 8
>> + at gepz = extern_weak global %gept
>> +
>> +define i1 @gep3() {
>> +; CHECK: @gep3
>> + %x = alloca %gept, align 8
>> + %a = getelementptr %gept* %x, i64 0, i32 0
>> + %b = getelementptr %gept* %x, i64 0, i32 1
>> + %equal = icmp eq i32* %a, %b
>> + ret i1 %equal
>> +; CHECK-NEXT: ret i1 false
>> +}
>> +
>> +define i1 @gep4() {
>> +; CHECK: @gep4
>> + %x = alloca %gept, align 8
>> + %a = getelementptr %gept* @gepy, i64 0, i32 0
>> + %b = getelementptr %gept* @gepy, i64 0, i32 1
>> + %equal = icmp eq i32* %a, %b
>> + ret i1 %equal
>> +; CHECK-NEXT: ret i1 false
>> +}
>> +
>> +define i1 @gep5() {
>> +; CHECK: @gep5
>> + %x = alloca %gept, align 8
>> + %a = getelementptr inbounds %gept* %x, i64 0, i32 1
>> + %b = getelementptr %gept* @gepy, i64 0, i32 0
>> + %equal = icmp eq i32* %a, %b
>> + ret i1 %equal
>> +; CHECK-NEXT: ret i1 false
>> +}
>> +
>> +define i1 @gep6(%gept* %x) {
>> +; Same as @gep3 but potentially null.
>> +; CHECK: @gep6
>> + %a = getelementptr %gept* %x, i64 0, i32 0
>> + %b = getelementptr %gept* %x, i64 0, i32 1
>> + %equal = icmp eq i32* %a, %b
>> + ret i1 %equal
>> +; CHECK-NEXT: ret i1 false
>> +}
>> +
>> +define i1 @gep7(%gept* %x) {
>> +; CHECK: @gep7
>> + %a = getelementptr %gept* %x, i64 0, i32 0
>> + %b = getelementptr %gept* @gepz, i64 0, i32 0
>> + %equal = icmp eq i32* %a, %b
>> + ret i1 %equal
>> +; CHECK: ret i1 %equal
>> +}
>> +
>> +define i1 @gep8(%gept* %x) {
>> +; CHECK: @gep8
>> + %a = getelementptr %gept* %x, i32 1
>> + %b = getelementptr %gept* %x, i32 -1
>> + %equal = icmp ugt %gept* %a, %b
>> + ret i1 %equal
>> +; CHECK: ret i1 %equal
>> +}
>> +
>> define i1 @zext(i32 %x) {
>> ; CHECK: @zext
>> %e1 = zext i32 %x to i64
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list