Skip to content

Commit a31ce36

Browse files
authored
Apply initializes attribute to DSE (#113630)
retry #107282 Fixed with `MadeChange |= Changed;` and confirmed it works. ``` cmake -DLLVM_CCACHE_BUILD=ON -DLLVM_ENABLE_EXPENSIVE_CHECKS=ON -DLLVM_ENABLE_WERROR=OFF -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_FLAGS=-U_GLIBCXX_DEBUG '-DLLVM_LIT_ARGS=-v -vv -j96' '-DLLVM_ENABLE_PROJECTS=llvm;lld' -DLLVM_ENABLE_ASSERTIONS=ON -GNinja ../llvm ninja check-llvm ```
1 parent 78e7d95 commit a31ce36

File tree

2 files changed

+510
-42
lines changed

2 files changed

+510
-42
lines changed

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

+209-42
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
#include "llvm/IR/Argument.h"
5353
#include "llvm/IR/BasicBlock.h"
5454
#include "llvm/IR/Constant.h"
55+
#include "llvm/IR/ConstantRangeList.h"
5556
#include "llvm/IR/Constants.h"
5657
#include "llvm/IR/DataLayout.h"
5758
#include "llvm/IR/DebugInfo.h"
@@ -164,6 +165,11 @@ static cl::opt<bool>
164165
OptimizeMemorySSA("dse-optimize-memoryssa", cl::init(true), cl::Hidden,
165166
cl::desc("Allow DSE to optimize memory accesses."));
166167

168+
// TODO: turn on and remove this flag.
169+
static cl::opt<bool> EnableInitializesImprovement(
170+
"enable-dse-initializes-attr-improvement", cl::init(false), cl::Hidden,
171+
cl::desc("Enable the initializes attr improvement in DSE"));
172+
167173
//===----------------------------------------------------------------------===//
168174
// Helper functions
169175
//===----------------------------------------------------------------------===//
@@ -809,8 +815,10 @@ bool canSkipDef(MemoryDef *D, bool DefVisibleToCaller) {
809815
// A memory location wrapper that represents a MemoryLocation, `MemLoc`,
810816
// defined by `MemDef`.
811817
struct MemoryLocationWrapper {
812-
MemoryLocationWrapper(MemoryLocation MemLoc, MemoryDef *MemDef)
813-
: MemLoc(MemLoc), MemDef(MemDef) {
818+
MemoryLocationWrapper(MemoryLocation MemLoc, MemoryDef *MemDef,
819+
bool DefByInitializesAttr)
820+
: MemLoc(MemLoc), MemDef(MemDef),
821+
DefByInitializesAttr(DefByInitializesAttr) {
814822
assert(MemLoc.Ptr && "MemLoc should be not null");
815823
UnderlyingObject = getUnderlyingObject(MemLoc.Ptr);
816824
DefInst = MemDef->getMemoryInst();
@@ -820,20 +828,59 @@ struct MemoryLocationWrapper {
820828
const Value *UnderlyingObject;
821829
MemoryDef *MemDef;
822830
Instruction *DefInst;
831+
bool DefByInitializesAttr = false;
823832
};
824833

825834
// A memory def wrapper that represents a MemoryDef and the MemoryLocation(s)
826835
// defined by this MemoryDef.
827836
struct MemoryDefWrapper {
828-
MemoryDefWrapper(MemoryDef *MemDef, std::optional<MemoryLocation> MemLoc) {
837+
MemoryDefWrapper(MemoryDef *MemDef,
838+
ArrayRef<std::pair<MemoryLocation, bool>> MemLocations) {
829839
DefInst = MemDef->getMemoryInst();
830-
if (MemLoc.has_value())
831-
DefinedLocation = MemoryLocationWrapper(*MemLoc, MemDef);
840+
for (auto &[MemLoc, DefByInitializesAttr] : MemLocations)
841+
DefinedLocations.push_back(
842+
MemoryLocationWrapper(MemLoc, MemDef, DefByInitializesAttr));
832843
}
833844
Instruction *DefInst;
834-
std::optional<MemoryLocationWrapper> DefinedLocation = std::nullopt;
845+
SmallVector<MemoryLocationWrapper, 1> DefinedLocations;
846+
};
847+
848+
bool hasInitializesAttr(Instruction *I) {
849+
CallBase *CB = dyn_cast<CallBase>(I);
850+
return CB && CB->getArgOperandWithAttribute(Attribute::Initializes);
851+
}
852+
853+
struct ArgumentInitInfo {
854+
unsigned Idx;
855+
bool IsDeadOrInvisibleOnUnwind;
856+
ConstantRangeList Inits;
835857
};
836858

859+
// Return the intersected range list of the initializes attributes of "Args".
860+
// "Args" are call arguments that alias to each other.
861+
// If any argument in "Args" doesn't have dead_on_unwind attr and
862+
// "CallHasNoUnwindAttr" is false, return empty.
863+
ConstantRangeList getIntersectedInitRangeList(ArrayRef<ArgumentInitInfo> Args,
864+
bool CallHasNoUnwindAttr) {
865+
if (Args.empty())
866+
return {};
867+
868+
// To address unwind, the function should have nounwind attribute or the
869+
// arguments have dead or invisible on unwind. Otherwise, return empty.
870+
for (const auto &Arg : Args) {
871+
if (!CallHasNoUnwindAttr && !Arg.IsDeadOrInvisibleOnUnwind)
872+
return {};
873+
if (Arg.Inits.empty())
874+
return {};
875+
}
876+
877+
ConstantRangeList IntersectedIntervals = Args.front().Inits;
878+
for (auto &Arg : Args.drop_front())
879+
IntersectedIntervals = IntersectedIntervals.intersectWith(Arg.Inits);
880+
881+
return IntersectedIntervals;
882+
}
883+
837884
struct DSEState {
838885
Function &F;
839886
AliasAnalysis &AA;
@@ -911,7 +958,8 @@ struct DSEState {
911958

912959
auto *MD = dyn_cast_or_null<MemoryDef>(MA);
913960
if (MD && MemDefs.size() < MemorySSADefsPerBlockLimit &&
914-
(getLocForWrite(&I) || isMemTerminatorInst(&I)))
961+
(getLocForWrite(&I) || isMemTerminatorInst(&I) ||
962+
(EnableInitializesImprovement && hasInitializesAttr(&I))))
915963
MemDefs.push_back(MD);
916964
}
917965
}
@@ -1147,13 +1195,26 @@ struct DSEState {
11471195
return MemoryLocation::getOrNone(I);
11481196
}
11491197

1150-
std::optional<MemoryLocation> getLocForInst(Instruction *I) {
1198+
// Returns a list of <MemoryLocation, bool> pairs written by I.
1199+
// The bool means whether the write is from Initializes attr.
1200+
SmallVector<std::pair<MemoryLocation, bool>, 1>
1201+
getLocForInst(Instruction *I, bool ConsiderInitializesAttr) {
1202+
SmallVector<std::pair<MemoryLocation, bool>, 1> Locations;
11511203
if (isMemTerminatorInst(I)) {
1152-
if (auto Loc = getLocForTerminator(I)) {
1153-
return Loc->first;
1204+
if (auto Loc = getLocForTerminator(I))
1205+
Locations.push_back(std::make_pair(Loc->first, false));
1206+
return Locations;
1207+
}
1208+
1209+
if (auto Loc = getLocForWrite(I))
1210+
Locations.push_back(std::make_pair(*Loc, false));
1211+
1212+
if (ConsiderInitializesAttr) {
1213+
for (auto &MemLoc : getInitializesArgMemLoc(I)) {
1214+
Locations.push_back(std::make_pair(MemLoc, true));
11541215
}
11551216
}
1156-
return getLocForWrite(I);
1217+
return Locations;
11571218
}
11581219

11591220
/// Assuming this instruction has a dead analyzable write, can we delete
@@ -1365,7 +1426,8 @@ struct DSEState {
13651426
getDomMemoryDef(MemoryDef *KillingDef, MemoryAccess *StartAccess,
13661427
const MemoryLocation &KillingLoc, const Value *KillingUndObj,
13671428
unsigned &ScanLimit, unsigned &WalkerStepLimit,
1368-
bool IsMemTerm, unsigned &PartialLimit) {
1429+
bool IsMemTerm, unsigned &PartialLimit,
1430+
bool IsInitializesAttrMemLoc) {
13691431
if (ScanLimit == 0 || WalkerStepLimit == 0) {
13701432
LLVM_DEBUG(dbgs() << "\n ... hit scan limit\n");
13711433
return std::nullopt;
@@ -1602,7 +1664,16 @@ struct DSEState {
16021664

16031665
// Uses which may read the original MemoryDef mean we cannot eliminate the
16041666
// original MD. Stop walk.
1605-
if (isReadClobber(MaybeDeadLoc, UseInst)) {
1667+
// If KillingDef is a CallInst with "initializes" attribute, the reads in
1668+
// the callee would be dominated by initializations, so it should be safe.
1669+
bool IsKillingDefFromInitAttr = false;
1670+
if (IsInitializesAttrMemLoc) {
1671+
if (KillingI == UseInst &&
1672+
KillingUndObj == getUnderlyingObject(MaybeDeadLoc.Ptr))
1673+
IsKillingDefFromInitAttr = true;
1674+
}
1675+
1676+
if (isReadClobber(MaybeDeadLoc, UseInst) && !IsKillingDefFromInitAttr) {
16061677
LLVM_DEBUG(dbgs() << " ... found read clobber\n");
16071678
return std::nullopt;
16081679
}
@@ -2171,6 +2242,16 @@ struct DSEState {
21712242
return MadeChange;
21722243
}
21732244

2245+
// Return the locations written by the initializes attribute.
2246+
// Note that this function considers:
2247+
// 1. Unwind edge: use "initializes" attribute only if the callee has
2248+
// "nounwind" attribute, or the argument has "dead_on_unwind" attribute,
2249+
// or the argument is invisible to caller on unwind. That is, we don't
2250+
// perform incorrect DSE on unwind edges in the current function.
2251+
// 2. Argument alias: for aliasing arguments, the "initializes" attribute is
2252+
// the intersected range list of their "initializes" attributes.
2253+
SmallVector<MemoryLocation, 1> getInitializesArgMemLoc(const Instruction *I);
2254+
21742255
// Try to eliminate dead defs that access `KillingLocWrapper.MemLoc` and are
21752256
// killed by `KillingLocWrapper.MemDef`. Return whether
21762257
// any changes were made, and whether `KillingLocWrapper.DefInst` was deleted.
@@ -2182,6 +2263,75 @@ struct DSEState {
21822263
bool eliminateDeadDefs(const MemoryDefWrapper &KillingDefWrapper);
21832264
};
21842265

2266+
SmallVector<MemoryLocation, 1>
2267+
DSEState::getInitializesArgMemLoc(const Instruction *I) {
2268+
const CallBase *CB = dyn_cast<CallBase>(I);
2269+
if (!CB)
2270+
return {};
2271+
2272+
// Collect aliasing arguments and their initializes ranges.
2273+
SmallMapVector<Value *, SmallVector<ArgumentInitInfo, 2>, 2> Arguments;
2274+
for (unsigned Idx = 0, Count = CB->arg_size(); Idx < Count; ++Idx) {
2275+
ConstantRangeList Inits;
2276+
Attribute InitializesAttr = CB->getParamAttr(Idx, Attribute::Initializes);
2277+
if (InitializesAttr.isValid())
2278+
Inits = InitializesAttr.getValueAsConstantRangeList();
2279+
2280+
Value *CurArg = CB->getArgOperand(Idx);
2281+
// We don't perform incorrect DSE on unwind edges in the current function,
2282+
// and use the "initializes" attribute to kill dead stores if:
2283+
// - The call does not throw exceptions, "CB->doesNotThrow()".
2284+
// - Or the callee parameter has "dead_on_unwind" attribute.
2285+
// - Or the argument is invisible to caller on unwind, and there are no
2286+
// unwind edges from this call in the current function (e.g. `CallInst`).
2287+
bool IsDeadOrInvisibleOnUnwind =
2288+
CB->paramHasAttr(Idx, Attribute::DeadOnUnwind) ||
2289+
(isa<CallInst>(CB) && isInvisibleToCallerOnUnwind(CurArg));
2290+
ArgumentInitInfo InitInfo{Idx, IsDeadOrInvisibleOnUnwind, Inits};
2291+
bool FoundAliasing = false;
2292+
for (auto &[Arg, AliasList] : Arguments) {
2293+
auto AAR = BatchAA.alias(MemoryLocation::getBeforeOrAfter(Arg),
2294+
MemoryLocation::getBeforeOrAfter(CurArg));
2295+
if (AAR == AliasResult::NoAlias) {
2296+
continue;
2297+
} else if (AAR == AliasResult::MustAlias) {
2298+
FoundAliasing = true;
2299+
AliasList.push_back(InitInfo);
2300+
} else {
2301+
// For PartialAlias and MayAlias, there is an offset or may be an
2302+
// unknown offset between the arguments and we insert an empty init
2303+
// range to discard the entire initializes info while intersecting.
2304+
FoundAliasing = true;
2305+
AliasList.push_back(ArgumentInitInfo{Idx, IsDeadOrInvisibleOnUnwind,
2306+
ConstantRangeList()});
2307+
}
2308+
}
2309+
if (!FoundAliasing)
2310+
Arguments[CurArg] = {InitInfo};
2311+
}
2312+
2313+
SmallVector<MemoryLocation, 1> Locations;
2314+
for (const auto &[_, Args] : Arguments) {
2315+
auto IntersectedRanges =
2316+
getIntersectedInitRangeList(Args, CB->doesNotThrow());
2317+
if (IntersectedRanges.empty())
2318+
continue;
2319+
2320+
for (const auto &Arg : Args) {
2321+
for (const auto &Range : IntersectedRanges) {
2322+
int64_t Start = Range.getLower().getSExtValue();
2323+
int64_t End = Range.getUpper().getSExtValue();
2324+
// For now, we only handle locations starting at offset 0.
2325+
if (Start == 0)
2326+
Locations.push_back(MemoryLocation(CB->getArgOperand(Arg.Idx),
2327+
LocationSize::precise(End - Start),
2328+
CB->getAAMetadata()));
2329+
}
2330+
}
2331+
}
2332+
return Locations;
2333+
}
2334+
21852335
std::pair<bool, bool>
21862336
DSEState::eliminateDeadDefs(const MemoryLocationWrapper &KillingLocWrapper) {
21872337
bool Changed = false;
@@ -2208,7 +2358,8 @@ DSEState::eliminateDeadDefs(const MemoryLocationWrapper &KillingLocWrapper) {
22082358
std::optional<MemoryAccess *> MaybeDeadAccess = getDomMemoryDef(
22092359
KillingLocWrapper.MemDef, Current, KillingLocWrapper.MemLoc,
22102360
KillingLocWrapper.UnderlyingObject, ScanLimit, WalkerStepLimit,
2211-
isMemTerminatorInst(KillingLocWrapper.DefInst), PartialLimit);
2361+
isMemTerminatorInst(KillingLocWrapper.DefInst), PartialLimit,
2362+
KillingLocWrapper.DefByInitializesAttr);
22122363

22132364
if (!MaybeDeadAccess) {
22142365
LLVM_DEBUG(dbgs() << " finished walk\n");
@@ -2231,10 +2382,20 @@ DSEState::eliminateDeadDefs(const MemoryLocationWrapper &KillingLocWrapper) {
22312382
}
22322383
continue;
22332384
}
2385+
// We cannot apply the initializes attribute to DeadAccess/DeadDef.
2386+
// It would incorrectly consider a call instruction as redundant store
2387+
// and remove this call instruction.
2388+
// TODO: this conflates the existence of a MemoryLocation with being able
2389+
// to delete the instruction. Fix isRemovable() to consider calls with
2390+
// side effects that cannot be removed, e.g. calls with the initializes
2391+
// attribute, and remove getLocForInst(ConsiderInitializesAttr = false).
22342392
MemoryDefWrapper DeadDefWrapper(
22352393
cast<MemoryDef>(DeadAccess),
2236-
getLocForInst(cast<MemoryDef>(DeadAccess)->getMemoryInst()));
2237-
MemoryLocationWrapper &DeadLocWrapper = *DeadDefWrapper.DefinedLocation;
2394+
getLocForInst(cast<MemoryDef>(DeadAccess)->getMemoryInst(),
2395+
/*ConsiderInitializesAttr=*/false));
2396+
assert(DeadDefWrapper.DefinedLocations.size() == 1);
2397+
MemoryLocationWrapper &DeadLocWrapper =
2398+
DeadDefWrapper.DefinedLocations.front();
22382399
LLVM_DEBUG(dbgs() << " (" << *DeadLocWrapper.DefInst << ")\n");
22392400
ToCheck.insert(DeadLocWrapper.MemDef->getDefiningAccess());
22402401
NumGetDomMemoryDefPassed++;
@@ -2309,37 +2470,42 @@ DSEState::eliminateDeadDefs(const MemoryLocationWrapper &KillingLocWrapper) {
23092470
}
23102471

23112472
bool DSEState::eliminateDeadDefs(const MemoryDefWrapper &KillingDefWrapper) {
2312-
if (!KillingDefWrapper.DefinedLocation.has_value()) {
2473+
if (KillingDefWrapper.DefinedLocations.empty()) {
23132474
LLVM_DEBUG(dbgs() << "Failed to find analyzable write location for "
23142475
<< *KillingDefWrapper.DefInst << "\n");
23152476
return false;
23162477
}
23172478

2318-
auto &KillingLocWrapper = *KillingDefWrapper.DefinedLocation;
2319-
LLVM_DEBUG(dbgs() << "Trying to eliminate MemoryDefs killed by "
2320-
<< *KillingLocWrapper.MemDef << " ("
2321-
<< *KillingLocWrapper.DefInst << ")\n");
2322-
auto [Changed, DeletedKillingLoc] = eliminateDeadDefs(KillingLocWrapper);
2323-
2324-
// Check if the store is a no-op.
2325-
if (!DeletedKillingLoc && storeIsNoop(KillingLocWrapper.MemDef,
2326-
KillingLocWrapper.UnderlyingObject)) {
2327-
LLVM_DEBUG(dbgs() << "DSE: Remove No-Op Store:\n DEAD: "
2328-
<< *KillingLocWrapper.DefInst << '\n');
2329-
deleteDeadInstruction(KillingLocWrapper.DefInst);
2330-
NumRedundantStores++;
2331-
return true;
2332-
}
2333-
// Can we form a calloc from a memset/malloc pair?
2334-
if (!DeletedKillingLoc &&
2335-
tryFoldIntoCalloc(KillingLocWrapper.MemDef,
2336-
KillingLocWrapper.UnderlyingObject)) {
2337-
LLVM_DEBUG(dbgs() << "DSE: Remove memset after forming calloc:\n"
2338-
<< " DEAD: " << *KillingLocWrapper.DefInst << '\n');
2339-
deleteDeadInstruction(KillingLocWrapper.DefInst);
2340-
return true;
2479+
bool MadeChange = false;
2480+
for (auto &KillingLocWrapper : KillingDefWrapper.DefinedLocations) {
2481+
LLVM_DEBUG(dbgs() << "Trying to eliminate MemoryDefs killed by "
2482+
<< *KillingLocWrapper.MemDef << " ("
2483+
<< *KillingLocWrapper.DefInst << ")\n");
2484+
auto [Changed, DeletedKillingLoc] = eliminateDeadDefs(KillingLocWrapper);
2485+
MadeChange |= Changed;
2486+
2487+
// Check if the store is a no-op.
2488+
if (!DeletedKillingLoc && storeIsNoop(KillingLocWrapper.MemDef,
2489+
KillingLocWrapper.UnderlyingObject)) {
2490+
LLVM_DEBUG(dbgs() << "DSE: Remove No-Op Store:\n DEAD: "
2491+
<< *KillingLocWrapper.DefInst << '\n');
2492+
deleteDeadInstruction(KillingLocWrapper.DefInst);
2493+
NumRedundantStores++;
2494+
MadeChange = true;
2495+
continue;
2496+
}
2497+
// Can we form a calloc from a memset/malloc pair?
2498+
if (!DeletedKillingLoc &&
2499+
tryFoldIntoCalloc(KillingLocWrapper.MemDef,
2500+
KillingLocWrapper.UnderlyingObject)) {
2501+
LLVM_DEBUG(dbgs() << "DSE: Remove memset after forming calloc:\n"
2502+
<< " DEAD: " << *KillingLocWrapper.DefInst << '\n');
2503+
deleteDeadInstruction(KillingLocWrapper.DefInst);
2504+
MadeChange = true;
2505+
continue;
2506+
}
23412507
}
2342-
return Changed;
2508+
return MadeChange;
23432509
}
23442510

23452511
static bool eliminateDeadStores(Function &F, AliasAnalysis &AA, MemorySSA &MSSA,
@@ -2355,7 +2521,8 @@ static bool eliminateDeadStores(Function &F, AliasAnalysis &AA, MemorySSA &MSSA,
23552521
continue;
23562522

23572523
MemoryDefWrapper KillingDefWrapper(
2358-
KillingDef, State.getLocForInst(KillingDef->getMemoryInst()));
2524+
KillingDef, State.getLocForInst(KillingDef->getMemoryInst(),
2525+
EnableInitializesImprovement));
23592526
MadeChange |= State.eliminateDeadDefs(KillingDefWrapper);
23602527
}
23612528

0 commit comments

Comments
 (0)