[llvm-commits] [llvm] r159202 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp test/Transforms/InstCombine/alloca.ll

[David Blaikie]

On Tue, Jun 26, 2012 at 6:39 AM, Duncan Sands <baldrick at free.fr> wrote:
> Author: baldrick
> Date: Tue Jun 26 08:39:21 2012
> New Revision: 159202
>
> URL: http://llvm.org/viewvc/llvm-project?rev=159202&view=rev
> Log:
> Replacing zero-sized alloca's with a null pointer is too aggressive, instead
> merge all zero-sized alloca's into one,

Is that sufficient? At least C++'s new is guaranteed to return a
unique (for its lifetime), undereferencable, pointer for each
zero-sized allocation. But I don't know how alloca's correlate to any
particular other language features that may or may not have similar
semantics.

- David

> fixing c43204g from the Ada ACATS
> conformance testsuite.  What happened there was that a variable sized object
> was being allocated on the stack, "alloca i8, i32 %size".  It was then being
> passed to another function, which tested that the address was not null (raising
> an exception if it was) then manipulated %size bytes in it (load and/or store).
> The optimizers cleverly managed to deduce that %size was zero (congratulations
> to them, as it isn't at all obvious), which made the alloca zero size, causing
> the optimizers to replace it with null, which then caused the check mentioned
> above to fail, and the exception to be raised, wrongly.  Note that no loads
> and stores were actually being done to the alloca (the loop that does them is
> executed %size times, i.e. is not executed), only the not-null address check.
>
> Modified:
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>    llvm/trunk/test/Transforms/InstCombine/alloca.ll
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=159202&r1=159201&r2=159202&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp Tue Jun 26 08:39:21 2012
> @@ -106,7 +106,6 @@
>     if (const ConstantInt *C = dyn_cast<ConstantInt>(AI.getArraySize())) {
>       Type *NewTy =
>         ArrayType::get(AI.getAllocatedType(), C->getZExtValue());
> -      assert(isa<AllocaInst>(AI) && "Unknown type of allocation inst!");
>       AllocaInst *New = Builder->CreateAlloca(NewTy, 0, AI.getName());
>       New->setAlignment(AI.getAlignment());
>
> @@ -135,16 +134,49 @@
>     }
>   }
>
> -  if (TD && isa<AllocaInst>(AI) && AI.getAllocatedType()->isSized()) {
> -    // If alloca'ing a zero byte object, replace the alloca with a null pointer.
> -    // Note that we only do this for alloca's, because malloc should allocate
> -    // and return a unique pointer, even for a zero byte allocation.
> -    if (TD->getTypeAllocSize(AI.getAllocatedType()) == 0)
> -      return ReplaceInstUsesWith(AI, Constant::getNullValue(AI.getType()));
> -
> +  if (TD && AI.getAllocatedType()->isSized()) {
>     // If the alignment is 0 (unspecified), assign it the preferred alignment.
>     if (AI.getAlignment() == 0)
>       AI.setAlignment(TD->getPrefTypeAlignment(AI.getAllocatedType()));
> +
> +    // Move all alloca's of zero byte objects to the entry block and merge them
> +    // together.  Note that we only do this for alloca's, because malloc should
> +    // allocate and return a unique pointer, even for a zero byte allocation.
> +    if (TD->getTypeAllocSize(AI.getAllocatedType()) == 0) {
> +      // For a zero sized alloca there is no point in doing an array allocation.
> +      // This is helpful if the array size is a complicated expression not used
> +      // elsewhere.
> +      if (AI.isArrayAllocation()) {
> +        AI.setOperand(0, ConstantInt::get(AI.getArraySize()->getType(), 1));
> +        return &AI;
> +      }
> +
> +      // Get the first instruction in the entry block.
> +      BasicBlock &EntryBlock = AI.getParent()->getParent()->getEntryBlock();
> +      Instruction *FirstInst = EntryBlock.getFirstNonPHIOrDbg();
> +      if (FirstInst != &AI) {
> +        // If the entry block doesn't start with a zero-size alloca then move
> +        // this one to the start of the entry block.  There is no problem with
> +        // dominance as the array size was forced to a constant earlier already.
> +        AllocaInst *EntryAI = dyn_cast<AllocaInst>(FirstInst);
> +        if (!EntryAI || !EntryAI->getAllocatedType()->isSized() ||
> +            TD->getTypeAllocSize(EntryAI->getAllocatedType()) != 0) {
> +          AI.moveBefore(FirstInst);
> +          return &AI;
> +        }
> +
> +        // Replace this zero-sized alloca with the one at the start of the entry
> +        // block after ensuring that the address will be aligned enough for both
> +        // types.
> +        unsigned MaxAlign =
> +          std::max(TD->getPrefTypeAlignment(EntryAI->getAllocatedType()),
> +                   TD->getPrefTypeAlignment(AI.getAllocatedType()));
> +        EntryAI->setAlignment(MaxAlign);
> +        if (AI.getType() != EntryAI->getType())
> +          return new BitCastInst(EntryAI, AI.getType());
> +        return ReplaceInstUsesWith(AI, EntryAI);
> +      }
> +    }
>   }
>
>   // Try to aggressively remove allocas which are only used for GEPs, lifetime
>
> Modified: llvm/trunk/test/Transforms/InstCombine/alloca.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/alloca.ll?rev=159202&r1=159201&r2=159202&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/alloca.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/alloca.ll Tue Jun 26 08:39:21 2012
> @@ -5,8 +5,11 @@
>
>  declare void @use(...)
>
> -; Zero byte allocas should be deleted.
> + at int = global i32 zeroinitializer
> +
> +; Zero byte allocas should be merged if they can't be deleted.
>  ; CHECK: @test
> +; CHECK: alloca
>  ; CHECK-NOT: alloca
>  define void @test() {
>         %X = alloca [0 x i32]           ; <[0 x i32]*> [#uses=1]
> @@ -15,6 +18,9 @@
>         call void (...)* @use( i32* %Y )
>         %Z = alloca {  }                ; <{  }*> [#uses=1]
>         call void (...)* @use( {  }* %Z )
> +        %size = load i32* @int
> +        %A = alloca {{}}, i32 %size
> +        call void (...)* @use( {{}}* %A )
>         ret void
>  }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits