[llvm] r176408 - recommit r172363 & r171325 (reverted in r172756)

Shuxin Yang shuxin.llvm at gmail.com
Sat Mar 2 14:21:48 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.


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
> +}
>
>
> _______________________________________________
> 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