[llvm] r214897 - Remove dead zero store to calloc initialized memory
Rui Ueyama
ruiu at google.com
Wed Aug 6 15:47:06 PDT 2014
I don't know the details of how to reproduce this easily, and the person
who did it is in a different timezone. I hope a followup will come soon.
I'm sorry for the inconvenience.
On Wed, Aug 6, 2014 at 3:43 PM, Philip Reames <listmail at philipreames.com>
wrote:
> I think I just spotted the error in this patch. I was checking that the
> defining instruction was a calloc, but I was not checking that the
> dependent instruction (the store) was actually storing to that calloc.
> Oops.
>
> I'll confirm this, prepare another version, and send it out for review.
>
> Are there easy instructions somewhere for replicating the msam build that
> failed?
>
>
> Philip
>
> On 08/06/2014 12:28 PM, Rui Ueyama wrote:
>
> We found that this patch seems to miscompile and confuse msan. I'm going
> to roll it back. I think the follow up comments about the issue will come
> from the people who know more on this.
>
>
> On Tue, Aug 5, 2014 at 10:48 AM, Philip Reames <listmail at philipreames.com>
> wrote:
>
>> Author: reames
>> Date: Tue Aug 5 12:48:20 2014
>> New Revision: 214897
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=214897&view=rev
>> Log:
>> Remove dead zero store to calloc initialized memory
>>
>> Optimize the following IR:
>>
>> %1 = tail call noalias i8* @calloc(i64 1, i64 4)
>> %2 = bitcast i8* %1 to i32*
>> ; This store is dead and should be removed
>> store i32 0, i32* %2, align 4
>>
>> Memory returned by calloc is guaranteed to be zero initialized. If the
>> value being stored is the constant zero (and the store is not otherwise
>> observable across threads), we can delete the store. If the store is to an
>> out of bounds address, it is undefined and thus also removable.
>>
>> Reviewed By: nicholas
>>
>> Differential Revision: http://reviews.llvm.org/D3942
>>
>>
>>
>> Added:
>> llvm/trunk/test/Transforms/DeadStoreElimination/calloc-store.ll
>> Modified:
>> llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp?rev=214897&r1=214896&r2=214897&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp Tue Aug 5
>> 12:48:20 2014
>> @@ -514,30 +514,50 @@ bool DSE::runOnBasicBlock(BasicBlock &BB
>> if (!InstDep.isDef() && !InstDep.isClobber())
>> continue;
>>
>> - // If we're storing the same value back to a pointer that we just
>> - // loaded from, then the store can be removed.
>> + // Check for cases where the store is redundant.
>> if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
>> + bool DeleteStore = false;
>> + // If we're storing the same value back to a pointer that we just
>> + // loaded from, then the store can be removed.
>> if (LoadInst *DepLoad = dyn_cast<LoadInst>(InstDep.getInst())) {
>> if (SI->getPointerOperand() == DepLoad->getPointerOperand() &&
>> SI->getOperand(0) == DepLoad && isRemovable(SI)) {
>> DEBUG(dbgs() << "DSE: Remove Store Of Load from same
>> pointer:\n "
>> << "LOAD: " << *DepLoad << "\n STORE: " << *SI
>> << '\n');
>> + DeleteStore = true;
>> + }
>> + }
>>
>> - // DeleteDeadInstruction can delete the current instruction.
>> Save BBI
>> - // in case we need it.
>> - WeakVH NextInst(BBI);
>> -
>> - DeleteDeadInstruction(SI, *MD, TLI);
>> -
>> - if (!NextInst) // Next instruction deleted.
>> - BBI = BB.begin();
>> - else if (BBI != BB.begin()) // Revisit this instruction if
>> possible.
>> - --BBI;
>> - ++NumFastStores;
>> - MadeChange = true;
>> - continue;
>> + // If we find a store to memory which was defined by calloc
>> + // we can remove the store if the value being stored is a
>> + // constant zero (since calloc initialized the memory to
>> + // that same value) or the store is undefined (if out of
>> + // bounds).
>> + if (isCallocLikeFn(InstDep.getInst(), TLI) && isRemovable(SI)) {
>> + Value *V = SI->getValueOperand();
>> + if (isa<Constant>(V) && cast<Constant>(V)->isNullValue()) {
>> + DEBUG(dbgs() << "DSE: Remove Store Of Zero to Calloc:\n "
>> + << "CALLOC: " << *InstDep.getInst() << "\n"
>> + << "STORE: " << *SI << '\n');
>> + DeleteStore = true;
>> }
>> }
>> +
>> + if (DeleteStore) {
>> + // DeleteDeadInstruction can delete the current instruction.
>> Save BBI
>> + // in case we need it.
>> + WeakVH NextInst(BBI);
>> +
>> + DeleteDeadInstruction(SI, *MD, TLI);
>> +
>> + if (!NextInst) // Next instruction deleted.
>> + BBI = BB.begin();
>> + else if (BBI != BB.begin()) // Revisit this instruction if
>> possible.
>> + --BBI;
>> + ++NumFastStores;
>> + MadeChange = true;
>> + continue;
>> + }
>> }
>>
>> // Figure out what location is being stored to.
>>
>> Added: llvm/trunk/test/Transforms/DeadStoreElimination/calloc-store.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/DeadStoreElimination/calloc-store.ll?rev=214897&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/DeadStoreElimination/calloc-store.ll
>> (added)
>> +++ llvm/trunk/test/Transforms/DeadStoreElimination/calloc-store.ll Tue
>> Aug 5 12:48:20 2014
>> @@ -0,0 +1,17 @@
>> +; RUN: opt < %s -basicaa -dse -S | FileCheck %s
>> +
>> +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"
>> +
>> +; Function Attrs: nounwind
>> +declare noalias i8* @calloc(i64, i64)
>> +
>> +; Function Attrs: nounwind uwtable
>> +define noalias i32* @test_store() {
>> +; CHECK-LABEL: test_store
>> + %1 = tail call noalias i8* @calloc(i64 1, i64 4)
>> + %2 = bitcast i8* %1 to i32*
>> + ; This store is dead and should be removed
>> + store i32 0, i32* %2, align 4
>> +; CHECK-NOT: store i32 0, i32* %2, align 4
>> + ret i32* %2
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140806/5924f21f/attachment.html>
More information about the llvm-commits
mailing list