Skip to content

Commit

Permalink
[DSE,MSSA] Treat store 0 after calloc as noop stores.
Browse files Browse the repository at this point in the history 
This patch extends storeIsNoop to also detect stores of 0 to an calloced
object. This basically ports the logic from legacy DSE to the MemorySSA
backed version.

It triggers in a few cases on MultiSource, SPEC2000, SPEC2006 with -O3
LTO:

Same hash: 218 (filtered out)
Remaining: 19
Metric: dse.NumNoopStores

Program                                        base   patch2 diff
 test-suite...CFP2000/177.mesa/177.mesa.test     1.00  15.00 1400.0%
 test-suite...6/482.sphinx3/482.sphinx3.test     1.00  14.00 1300.0%
 test-suite...lications/ClamAV/clamscan.test     2.00  28.00 1300.0%
 test-suite...CFP2006/433.milc/433.milc.test     1.00   8.00 700.0%
 test-suite...pplications/oggenc/oggenc.test     2.00   9.00 350.0%
 test-suite.../CINT2000/176.gcc/176.gcc.test     6.00   6.00  0.0%
 test-suite.../CINT2006/403.gcc/403.gcc.test    NaN   137.00  nan%
 test-suite...libquantum/462.libquantum.test    NaN     3.00  nan%
 test-suite...6/464.h264ref/464.h264ref.test    NaN     7.00  nan%
 test-suite...decode/alacconvert-decode.test    NaN     2.00  nan%
 test-suite...encode/alacconvert-encode.test    NaN     2.00  nan%
 test-suite...ications/JM/ldecod/ldecod.test    NaN     9.00  nan%
 test-suite...ications/JM/lencod/lencod.test    NaN    39.00  nan%
 test-suite.../Applications/lemon/lemon.test    NaN     2.00  nan%
 test-suite...pplications/treecc/treecc.test    NaN     4.00  nan%
 test-suite...hmarks/McCat/08-main/main.test    NaN     4.00  nan%
 test-suite...nsumer-lame/consumer-lame.test    NaN     3.00  nan%
 test-suite.../Prolangs-C/bison/mybison.test    NaN     1.00  nan%
 test-suite...arks/mafft/pairlocalalign.test    NaN    30.00  nan%

Reviewers: efriedma, zoecarver, asbirlea

Reviewed By: asbirlea

Differential Revision: https://reviews.llvm.org/D82204
  • Loading branch information
@fhahn
fhahn committed Jun 23, 2020
1 parent 3a55a2a commit ff4de86
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 38 deletions.
66 changes: 40 additions & 26 deletions llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
View file
Expand Up @@ -1922,22 +1922,38 @@ struct DSEState {
}
return false;
}
};

/// \returns true if \p KillingDef stores the result of \p Load to the source of
/// \p Load.
static bool storeIsNoop(MemorySSA &MSSA, LoadInst *Load,
MemoryDef *KillingDef) {
Instruction *Store = KillingDef->getMemoryInst();
// If the load's operand isn't the destination of the store, bail.
if (Load->getPointerOperand() != Store->getOperand(1))
return false;
/// \returns true if \p Def is a no-op store, either because it
/// directly stores back a loaded value or stores zero to a calloced object.
bool storeIsNoop(MemoryDef *Def, MemoryLocation DefLoc, const Value *DefUO) {
StoreInst *Store = dyn_cast<StoreInst>(Def->getMemoryInst());
if (!Store)
return false;

// Get the defining access for the load.
auto *LoadAccess = MSSA.getMemoryAccess(Load)->getDefiningAccess();
// The store is dead if the defining accesses are the same.
return LoadAccess == KillingDef->getDefiningAccess();
}
if (auto *LoadI = dyn_cast<LoadInst>(Store->getOperand(0))) {
if (LoadI->getPointerOperand() == Store->getOperand(1)) {
auto *LoadAccess = MSSA.getMemoryAccess(LoadI)->getDefiningAccess();
// If both accesses share the same defining access, no instructions
// between them can modify the memory location.
return LoadAccess == Def->getDefiningAccess();
}
}

Constant *StoredConstant = dyn_cast<Constant>(Store->getOperand(0));
if (StoredConstant && StoredConstant->isNullValue()) {
auto *DefUOInst = dyn_cast<Instruction>(DefUO);
if (DefUOInst && isCallocLikeFn(DefUOInst, &TLI)) {
auto *UnderlyingDef = cast<MemoryDef>(MSSA.getMemoryAccess(DefUOInst));
// If UnderlyingDef is the clobbering access of Def, no instructions
// between them can modify the memory location.
auto *ClobberDef =
MSSA.getSkipSelfWalker()->getClobberingMemoryAccess(Def);
return UnderlyingDef == ClobberDef;
}
}
return false;
}
};

bool eliminateDeadStoresMemorySSA(Function &F, AliasAnalysis &AA,
MemorySSA &MSSA, DominatorTree &DT,
Expand All @@ -1954,18 +1970,6 @@ bool eliminateDeadStoresMemorySSA(Function &F, AliasAnalysis &AA,
continue;
Instruction *SI = KillingDef->getMemoryInst();

// Check if we're storing a value that we just loaded.
if (auto *Load = dyn_cast<LoadInst>(SI->getOperand(0))) {
if (storeIsNoop(MSSA, Load, KillingDef)) {
LLVM_DEBUG(dbgs() << "DSE: Remove Dead Store:\n DEAD: " << *SI
<< '\n');
State.deleteDeadInstruction(SI);
NumNoopStores++;
MadeChange = true;
continue;
}
}

auto MaybeSILoc = State.getLocForWriteEx(SI);
if (!MaybeSILoc) {
LLVM_DEBUG(dbgs() << "Failed to find analyzable write location for "
Expand All @@ -1975,6 +1979,16 @@ bool eliminateDeadStoresMemorySSA(Function &F, AliasAnalysis &AA,
MemoryLocation SILoc = *MaybeSILoc;
assert(SILoc.Ptr && "SILoc should not be null");
const Value *SILocUnd = GetUnderlyingObject(SILoc.Ptr, DL);

// Check if the store is a no-op.
if (State.storeIsNoop(KillingDef, SILoc, SILocUnd)) {
LLVM_DEBUG(dbgs() << "DSE: Remove No-Op Store:\n DEAD: " << *SI << '\n');
State.deleteDeadInstruction(SI);
NumNoopStores++;
MadeChange = true;
continue;
}

Instruction *DefObj =
const_cast<Instruction *>(dyn_cast<Instruction>(SILocUnd));
bool DefVisibleToCallerBeforeRet =
Expand Down
97 changes: 95 additions & 2 deletions llvm/test/Transforms/DeadStoreElimination/MSSA/calloc-store.ll
View file
@@ -1,5 +1,3 @@
; XFAIL: *

; RUN: opt < %s -basicaa -dse -enable-dse-memoryssa -S | FileCheck %s

declare noalias i8* @calloc(i64, i64)
Expand Down Expand Up @@ -65,3 +63,98 @@ define i8* @test7(i8 %arg) {
; CHECK: store i8 %arg, i8* %1, align 4
ret i8* %1
}

define i8* @test8() {
; CHECK-LABEL: test8
; CHECK-NOT: store
%p = tail call noalias i8* @calloc(i64 1, i64 4)
store i8 0, i8* %p, align 1
%p.1 = getelementptr i8, i8* %p, i32 1
store i8 0, i8* %p.1, align 1
%p.3 = getelementptr i8, i8* %p, i32 3
store i8 0, i8* %p.3, align 1
%p.2 = getelementptr i8, i8* %p, i32 2
store i8 0, i8* %p.2, align 1
ret i8* %p
}

define i8* @test9() {
; CHECK-LABEL: test9
; CHECK-NEXT: %p = tail call noalias i8* @calloc(i64 1, i64 4)
; CHECK-NEXT: store i8 5, i8* %p, align 1
; CHECK-NEXT: ret i8* %p

%p = tail call noalias i8* @calloc(i64 1, i64 4)
store i8 5, i8* %p, align 1
%p.1 = getelementptr i8, i8* %p, i32 1
store i8 0, i8* %p.1, align 1
%p.3 = getelementptr i8, i8* %p, i32 3
store i8 0, i8* %p.3, align 1
%p.2 = getelementptr i8, i8* %p, i32 2
store i8 0, i8* %p.2, align 1
ret i8* %p
}

define i8* @test10() {
; CHECK-LABEL: @test10(
; CHECK-NEXT: [[P:%.*]] = tail call noalias i8* @calloc(i64 1, i64 4)
; CHECK-NEXT: [[P_3:%.*]] = getelementptr i8, i8* [[P]], i32 3
; CHECK-NEXT: store i8 5, i8* [[P_3]], align 1
; CHECK-NEXT: ret i8* [[P]]
;

%p = tail call noalias i8* @calloc(i64 1, i64 4)
store i8 0, i8* %p, align 1
%p.1 = getelementptr i8, i8* %p, i32 1
store i8 0, i8* %p.1, align 1
%p.3 = getelementptr i8, i8* %p, i32 3
store i8 5, i8* %p.3, align 1
%p.2 = getelementptr i8, i8* %p, i32 2
store i8 0, i8* %p.2, align 1
ret i8* %p
}

; TODO: we could also eliminate the last store i8 0, i8* %p.3.2, but currently
; don't because those are eliminated before eliminating killed stores.
define i8* @test11() {
; CHECK-LABEL: @test11(
; CHECK-NEXT: [[P:%.*]] = tail call noalias i8* @calloc(i64 1, i64 4)
; CHECK-NEXT: [[P_3_2:%.*]] = getelementptr i8, i8* [[P]], i32 3
; CHECK-NEXT: store i8 0, i8* [[P_3_2]], align 1
; CHECK-NEXT: ret i8* [[P]]
;

%p = tail call noalias i8* @calloc(i64 1, i64 4)
store i8 0, i8* %p, align 1
%p.1 = getelementptr i8, i8* %p, i32 1
store i8 0, i8* %p.1, align 1
%p.3 = getelementptr i8, i8* %p, i32 3
store i8 5, i8* %p.3, align 1
%p.2 = getelementptr i8, i8* %p, i32 2
store i8 0, i8* %p.2, align 1
%p.3.2 = getelementptr i8, i8* %p, i32 3
store i8 0, i8* %p.3.2, align 1
ret i8* %p
}

define i8* @test12() {
; CHECK-LABEL: @test12(
; CHECK-NEXT: [[P:%.*]] = tail call noalias i8* @calloc(i64 1, i64 4)
; CHECK-NEXT: [[P_3:%.*]] = getelementptr i8, i8* [[P]], i32 3
; CHECK-NEXT: store i8 5, i8* [[P_3]], align 1
; CHECK-NEXT: call void @use(i8* [[P]])
; CHECK-NEXT: [[P_3_2:%.*]] = getelementptr i8, i8* [[P]], i32 3
; CHECK-NEXT: store i8 0, i8* [[P_3_2]], align 1
; CHECK-NEXT: ret i8* [[P]]
;

%p = tail call noalias i8* @calloc(i64 1, i64 4)
%p.3 = getelementptr i8, i8* %p, i32 3
store i8 5, i8* %p.3, align 1
call void @use(i8* %p)
%p.3.2 = getelementptr i8, i8* %p, i32 3
store i8 0, i8* %p.3.2, align 1
ret i8* %p
}

declare void @use(i8*) readonly
10 changes: 0 additions & 10 deletions llvm/test/Transforms/DeadStoreElimination/MSSA/simple-todo.ll
View file
Expand Up @@ -55,7 +55,6 @@ define void @test11() {


declare noalias i8* @malloc(i32)
declare noalias i8* @calloc(i32, i32)

define void @test14(i32* %Q) {
; CHECK-LABEL: @test14(
Expand All @@ -77,15 +76,6 @@ define void @test20() {
ret void
}

define void @test21() {
; CHECK-LABEL: @test21(
; CHECK-NEXT: ret void
;
%m = call i8* @calloc(i32 9, i32 7)
store i8 0, i8* %m
ret void
}

define void @test22(i1 %i, i32 %k, i32 %m) nounwind {
; CHECK-LABEL: @test22(
; CHECK-NEXT: ret void
Expand Down
9 changes: 9 additions & 0 deletions llvm/test/Transforms/DeadStoreElimination/MSSA/simple.ll
View file
Expand Up @@ -213,6 +213,15 @@ bb:

}

define void @test21() {
; CHECK-LABEL: @test21(
; CHECK-NEXT: ret void
;
%m = call i8* @calloc(i32 9, i32 7)
store i8 0, i8* %m
ret void
}

; PR13547
declare noalias i8* @strdup(i8* nocapture) nounwind
define noalias i8* @test23() nounwind uwtable ssp {
Expand Down

0 comments on commit ff4de86

Please sign in to comment.