Skip to content

Commit 8e85d60

Browse files
committed
[pmo] A copy_addr of a trivial type should be treated as an InitOrAssign of the dest rather than like a non-trivial type.
PMO uses InitOrAssign for trivially typed things and Init/Assign for non-trivial things, so I think this was an oversight from a long time ago. There is actually no /real/ effect on the code today since after exploding the copy_addr, the store will still be used to produce the right available value and since for stores, init/assign/initorassign all result in allocations being removed. Once though I change assign to not allow for allocation removal (the proper way to model this), without this change, certain trivial allocations will no longer be removed, harming perf as seen via the benchmarking run on the bots in #21918.
1 parent 528fe6a commit 8e85d60

File tree

2 files changed

+3
-1
lines changed

2 files changed

+3
-1
lines changed

lib/SILOptimizer/Mandatory/PMOMemoryUseCollector.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,8 @@ bool ElementUseCollector::collectUses(SILValue Pointer) {
294294
auto Kind = ([&]() -> PMOUseKind {
295295
if (UI->getOperandNumber() == CopyAddrInst::Src)
296296
return PMOUseKind::Load;
297+
if (PointeeType.isTrivial(CAI->getModule()))
298+
return PMOUseKind::InitOrAssign;
297299
if (CAI->isInitializationOfDest())
298300
return PMOUseKind::Initialization;
299301
return PMOUseKind::Assign;

lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -911,7 +911,7 @@ void AvailableValueDataflowContext::explodeCopyAddr(CopyAddrInst *CAI) {
911911
assert((LoadUse.isValid() || StoreUse.isValid()) &&
912912
"we should have a load or a store, possibly both");
913913
assert(StoreUse.isInvalid() || StoreUse.Kind == Assign ||
914-
StoreUse.Kind == Initialization);
914+
StoreUse.Kind == Initialization || StoreUse.Kind == InitOrAssign);
915915

916916
// Now that we've emitted a bunch of instructions, including a load and store
917917
// but also including other stuff, update the internal state of

0 commit comments

Comments
 (0)