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

[Duncan Sands]

Hi David,

>> 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.

I don't know either for C++, but I think it suffices for Ada.

Ciao, Duncan.

>
> - 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