[llvm] r214897 - Remove dead zero store to calloc initialized memory
Philip Reames
listmail at philipreames.com
Wed Aug 6 15:06:14 PDT 2014
Feel free to revert. If you have an example that miscompiles, I'd be
greatly interested.
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 <mailto: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 <mailto: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/c4db67dd/attachment.html>
More information about the llvm-commits
mailing list