[llvm] r176408 - recommit r172363 & r171325 (reverted in r172756)
Nuno Lopes
nunoplopes at sapo.pt
Sun Mar 3 09:30:35 PST 2013
>I emphasize that your problematic should not resurrected without addressing
>my concerns I put
> in PR14988. I'm rather disappointed that you still go ahead resurrect your
> change.
>
> The ObjectSizeOffsetVisitor::visitPhiNode() dosen't guarantee if all
> operands points to the same
> objects. While it would not lead to problem if the function is called in
> the context of alias analysis.
> It would cause problem when instcombine try to replace llvm.objectsize()
> to constant.
I don't think that's true.
It is correct that visitPhiNode() can return a known size if the phi node
may point to multiple objects. But that will only happen if *all* those
objects have exactly the same size.
When instcombine folds llvm.objectsize() to a constant, it doesn't need to
know *exactly* to which objects its argument is pointing to. But if all the
pointed-to objects have the same size, then you can still fold
llvm.objectsize() to a constant, since you're sure that whaetever object it
will point to at run time, it will have the size you computed.
But please send an example if you believe I'm wrong.
Thanks,
Nuno
P.S.: BTW, thanks for you analysis you did for PR14988. I forgot to quote
you in the commit message, sorry.
> in 03/02/2013 03:36 AM, Nuno Lopes wrote:
>> Author: nlopes
>> Date: Sat Mar 2 05:36:24 2013
>> New Revision: 176408
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=176408&view=rev
>> Log:
>> recommit r172363 & r171325 (reverted in r172756)
>> This adds minimalistic support for PHI nodes to llvm.objectsize()
>> evaluation
>>
>> fingers crossed so that it does break clang boostrap again..
>>
>> Modified:
>> llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h
>> llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
>> llvm/trunk/test/Transforms/InstCombine/objsize.ll
>>
>> Modified: llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h?rev=176408&r1=176407&r2=176408&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h (original)
>> +++ llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h Sat Mar 2 05:36:24
>> 2013
>> @@ -161,12 +161,14 @@ typedef std::pair<APInt, APInt> SizeOffs
>> class ObjectSizeOffsetVisitor
>> : public InstVisitor<ObjectSizeOffsetVisitor, SizeOffsetType> {
>> + typedef DenseMap<const Value*, SizeOffsetType> CacheMapTy;
>> +
>> const DataLayout *TD;
>> const TargetLibraryInfo *TLI;
>> bool RoundToAlign;
>> unsigned IntTyBits;
>> APInt Zero;
>> - SmallPtrSet<Instruction *, 8> SeenInsts;
>> + CacheMapTy CacheMap;
>> APInt align(APInt Size, uint64_t Align);
>>
>> Modified: llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryBuiltins.cpp?rev=176408&r1=176407&r2=176408&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/MemoryBuiltins.cpp (original)
>> +++ llvm/trunk/lib/Analysis/MemoryBuiltins.cpp Sat Mar 2 05:36:24 2013
>> @@ -405,16 +405,23 @@ ObjectSizeOffsetVisitor::ObjectSizeOffse
>> SizeOffsetType ObjectSizeOffsetVisitor::compute(Value *V) {
>> V = V->stripPointerCasts();
>> - if (Instruction *I = dyn_cast<Instruction>(V)) {
>> - // If we have already seen this instruction, bail out. Cycles can
>> happen in
>> - // unreachable code after constant propagation.
>> - if (!SeenInsts.insert(I))
>> - return unknown();
>> + if (isa<Instruction>(V) || isa<GEPOperator>(V)) {
>> + // Return cached value or insert unknown in cache if size of V was
>> not
>> + // computed yet in order to avoid recursions in PHis.
>> + std::pair<CacheMapTy::iterator, bool> CacheVal =
>> + CacheMap.insert(std::make_pair(V, unknown()));
>> + if (!CacheVal.second)
>> + return CacheVal.first->second;
>> +
>> + SizeOffsetType Result;
>> if (GEPOperator *GEP = dyn_cast<GEPOperator>(V))
>> - return visitGEPOperator(*GEP);
>> - return visit(*I);
>> + Result = visitGEPOperator(*GEP);
>> + else
>> + Result = visit(cast<Instruction>(*V));
>> + return CacheMap[V] = Result;
>> }
>> +
>> if (Argument *A = dyn_cast<Argument>(V))
>> return visitArgument(*A);
>> if (ConstantPointerNull *P = dyn_cast<ConstantPointerNull>(V))
>> @@ -428,8 +435,6 @@ SizeOffsetType ObjectSizeOffsetVisitor::
>> if (ConstantExpr *CE = dyn_cast<ConstantExpr>(V)) {
>> if (CE->getOpcode() == Instruction::IntToPtr)
>> return unknown(); // clueless
>> - if (CE->getOpcode() == Instruction::GetElementPtr)
>> - return visitGEPOperator(cast<GEPOperator>(*CE));
>> }
>> DEBUG(dbgs() << "ObjectSizeOffsetVisitor::compute() unhandled value: "
>> << *V
>> @@ -563,9 +568,21 @@ SizeOffsetType ObjectSizeOffsetVisitor::
>> return unknown();
>> }
>> -SizeOffsetType ObjectSizeOffsetVisitor::visitPHINode(PHINode&) {
>> - // too complex to analyze statically.
>> - return unknown();
>> +SizeOffsetType ObjectSizeOffsetVisitor::visitPHINode(PHINode &PHI) {
>> + if (PHI.getNumIncomingValues() == 0)
>> + return unknown();
>> +
>> + SizeOffsetType Ret = compute(PHI.getIncomingValue(0));
>> + if (!bothKnown(Ret))
>> + return unknown();
>> +
>> + // Verify that all PHI incoming pointers have the same size and
>> offset.
>> + for (unsigned i = 1, e = PHI.getNumIncomingValues(); i != e; ++i) {
>> + SizeOffsetType EdgeData = compute(PHI.getIncomingValue(i));
>> + if (!bothKnown(EdgeData) || EdgeData != Ret)
>> + return unknown();
>> + }
>> + return Ret;
>> }
>> SizeOffsetType ObjectSizeOffsetVisitor::visitSelectInst(SelectInst &I)
>> {
>>
>> Modified: llvm/trunk/test/Transforms/InstCombine/objsize.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/objsize.ll?rev=176408&r1=176407&r2=176408&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/objsize.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/objsize.ll Sat Mar 2 05:36:24
>> 2013
>> @@ -256,3 +256,131 @@ xpto:
>> return:
>> ret i32 7
>> }
>> +
>> +declare noalias i8* @valloc(i32) nounwind
>> +
>> +; CHECK: @test14
>> +; CHECK: ret i32 6
>> +define i32 @test14(i32 %a) nounwind {
>> + switch i32 %a, label %sw.default [
>> + i32 1, label %sw.bb
>> + i32 2, label %sw.bb1
>> + ]
>> +
>> +sw.bb:
>> + %call = tail call noalias i8* @malloc(i32 6) nounwind
>> + br label %sw.epilog
>> +
>> +sw.bb1:
>> + %call2 = tail call noalias i8* @calloc(i32 3, i32 2) nounwind
>> + br label %sw.epilog
>> +
>> +sw.default:
>> + %call3 = tail call noalias i8* @valloc(i32 6) nounwind
>> + br label %sw.epilog
>> +
>> +sw.epilog:
>> + %b.0 = phi i8* [ %call3, %sw.default ], [ %call2, %sw.bb1 ], [ %call,
>> %sw.bb ]
>> + %1 = tail call i32 @llvm.objectsize.i32(i8* %b.0, i1 false)
>> + ret i32 %1
>> +}
>> +
>> +; CHECK: @test15
>> +; CHECK: llvm.objectsize
>> +define i32 @test15(i32 %a) nounwind {
>> + switch i32 %a, label %sw.default [
>> + i32 1, label %sw.bb
>> + i32 2, label %sw.bb1
>> + ]
>> +
>> +sw.bb:
>> + %call = tail call noalias i8* @malloc(i32 3) nounwind
>> + br label %sw.epilog
>> +
>> +sw.bb1:
>> + %call2 = tail call noalias i8* @calloc(i32 2, i32 1) nounwind
>> + br label %sw.epilog
>> +
>> +sw.default:
>> + %call3 = tail call noalias i8* @valloc(i32 3) nounwind
>> + br label %sw.epilog
>> +
>> +sw.epilog:
>> + %b.0 = phi i8* [ %call3, %sw.default ], [ %call2, %sw.bb1 ], [ %call,
>> %sw.bb ]
>> + %1 = tail call i32 @llvm.objectsize.i32(i8* %b.0, i1 false)
>> + ret i32 %1
>> +}
>> +
>> +; CHECK: @test16
>> +; CHECK: llvm.objectsize
>> +define i32 @test16(i8* %a, i32 %n) nounwind {
>> + %b = alloca [5 x i8], align 1
>> + %c = alloca [5 x i8], align 1
>> + switch i32 %n, label %sw.default [
>> + i32 1, label %sw.bb
>> + i32 2, label %sw.bb1
>> + ]
>> +
>> +sw.bb:
>> + %bp = bitcast [5 x i8]* %b to i8*
>> + br label %sw.epilog
>> +
>> +sw.bb1:
>> + %cp = bitcast [5 x i8]* %c to i8*
>> + br label %sw.epilog
>> +
>> +sw.default:
>> + br label %sw.epilog
>> +
>> +sw.epilog:
>> + %phi = phi i8* [ %a, %sw.default ], [ %cp, %sw.bb1 ], [ %bp, %sw.bb ]
>> + %sz = call i32 @llvm.objectsize.i32(i8* %phi, i1 false)
>> + ret i32 %sz
>> +}
>> +
>> +; CHECK: @test17
>> +; CHECK: ret i32 5
>> +define i32 @test17(i32 %n) nounwind {
>> + %b = alloca [5 x i8], align 1
>> + %c = alloca [5 x i8], align 1
>> + %bp = bitcast [5 x i8]* %b to i8*
>> + switch i32 %n, label %sw.default [
>> + i32 1, label %sw.bb
>> + i32 2, label %sw.bb1
>> + ]
>> +
>> +sw.bb:
>> + br label %sw.epilog
>> +
>> +sw.bb1:
>> + %cp = bitcast [5 x i8]* %c to i8*
>> + br label %sw.epilog
>> +
>> +sw.default:
>> + br label %sw.epilog
>> +
>> +sw.epilog:
>> + %phi = phi i8* [ %bp, %sw.default ], [ %cp, %sw.bb1 ], [ %bp, %sw.bb ]
>> + %sz = call i32 @llvm.objectsize.i32(i8* %phi, i1 false)
>> + ret i32 %sz
>> +}
>> +
>> + at globalalias = alias internal [60 x i8]* @a
>> +
>> +; CHECK: @test18
>> +; CHECK-NEXT: ret i32 60
>> +define i32 @test18() {
>> + %bc = bitcast [60 x i8]* @globalalias to i8*
>> + %1 = call i32 @llvm.objectsize.i32(i8* %bc, i1 false)
>> + ret i32 %1
>> +}
>> +
>> + at globalalias2 = alias weak [60 x i8]* @a
>> +
>> +; CHECK: @test19
>> +; CHECK: llvm.objectsize
>> +define i32 @test19() {
>> + %bc = bitcast [60 x i8]* @globalalias2 to i8*
>> + %1 = call i32 @llvm.objectsize.i32(i8* %bc, i1 false)
>> + ret i32 %1
>> +}
>>
More information about the llvm-commits
mailing list