Skip to content

Commit 9074a1e

Browse files
authored
Merge pull request #85005 from eeckstein/remove-copy-forwarding
Optimizer: remove the CopyForwarding pass
2 parents f095990 + 7a70049 commit 9074a1e

File tree

17 files changed

+84
-2004
lines changed

17 files changed

+84
-2004
lines changed

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/TempRValueElimination.swift

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,11 @@ private func getRemovableAllocStackDestination(
160160
// %x = load %allocStack // looks like a load, but is a `load [take]`
161161
// strong_release %x
162162
// ```
163-
guard copy.parentFunction.hasOwnership || allocStack.isDestroyedOnAllPaths(context) else {
163+
guard copy.parentFunction.hasOwnership ||
164+
allocStack.isDestroyedOnAllPaths(context) ||
165+
// We can easily remove a dead alloc_stack
166+
allocStack.uses.ignore(user: copy).ignore(usersOfType: DeallocStackInst.self).isEmpty
167+
else {
164168
return nil
165169
}
166170

@@ -177,6 +181,10 @@ private func hoistDestroy(of allocStack: AllocStackInst, after lastUse: Instruct
177181
case let li as LoadInst where li.loadOwnership == .take:
178182
assert(li.address == allocStack, "load must be not take a projected address")
179183
return
184+
case let apply as ApplyInst where apply.consumes(address: allocStack):
185+
if allocStack.uses.contains(where: { $0.instruction == apply && apply.convention(of: $0)!.isGuaranteed }) {
186+
return
187+
}
180188
default:
181189
break
182190
}
@@ -266,6 +274,14 @@ private extension AllocStackInst {
266274
}
267275
}
268276

277+
private extension ApplySite {
278+
func consumes(address: Value) -> Bool {
279+
argumentOperands.contains { argOp in
280+
argOp.value == address && convention(of: argOp)!.isConsumed
281+
}
282+
}
283+
}
284+
269285
/// Tries to move an `end_access` down to extend the access scope over all uses of the `alloc_stack`.
270286
/// For example:
271287
/// ```
@@ -531,13 +547,21 @@ private struct UseCollector : AddressDefUseWalker {
531547
uses.insert(copyFromStack)
532548
return .continueWalk
533549

550+
case is DebugValueInst:
551+
return .continueWalk
552+
534553
default:
535554
return .abortWalk
536555
}
537556
}
538557

539558
private mutating func visitApply(address: Operand, apply: ApplySite) -> WalkResult {
540-
if !apply.convention(of: address)!.isGuaranteed {
559+
let argConvention = apply.convention(of: address)!
560+
guard argConvention.isGuaranteed ||
561+
// Only accept consuming-in arguments if it consumes the whole `alloc_stack`. A consume from
562+
// a projection would destroy only a part of the `alloc_stack` and we don't handle this.
563+
(argConvention == .indirectIn && (copy.isTakeOfSource && address.value == copy.destinationAddress))
564+
else {
541565
return .abortWalk
542566
}
543567
uses.insert(apply)

include/swift/SILOptimizer/PassManager/Passes.def

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,6 @@ LEGACY_PASS(ConstantEvaluatorTester, "test-constant-evaluator",
254254
"Test constant evaluator")
255255
LEGACY_PASS(ConstantEvaluableSubsetChecker, "test-constant-evaluable-subset",
256256
"Test Swift code snippets expected to be constant evaluable")
257-
LEGACY_PASS(CopyForwarding, "copy-forwarding",
258-
"Copy Forwarding to Remove Redundant Copies")
259257
LEGACY_PASS(CopyPropagation, "copy-propagation",
260258
"Copy propagation to Remove Redundant SSA Copies, pruning debug info")
261259
LEGACY_PASS(MandatoryCopyPropagation, "mandatory-copy-propagation",

lib/SIL/Utils/InstructionUtils.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -841,7 +841,9 @@ RuntimeEffect swift::getRuntimeEffect(SILInstruction *inst, SILType &impactType)
841841
return RuntimeEffect::MetaData | RuntimeEffect::Releasing;
842842
if (!ca->isTakeOfSrc())
843843
return RuntimeEffect::MetaData | RuntimeEffect::RefCounting;
844-
return RuntimeEffect::MetaData;
844+
if (ca->getSrc()->getType().hasArchetype())
845+
return RuntimeEffect::MetaData;
846+
return RuntimeEffect::NoEffect;
845847
}
846848
case SILInstructionKind::TupleAddrConstructorInst: {
847849
auto *ca = cast<TupleAddrConstructorInst>(inst);

lib/SILOptimizer/PassManager/PassPipeline.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -459,12 +459,6 @@ void addFunctionPasses(SILPassPipelinePlan &P,
459459
P.addDestroyAddrHoisting();
460460
}
461461

462-
// Propagate copies through stack locations. Should run after
463-
// box-to-stack promotion since it is limited to propagating through
464-
// stack locations. Should run before aggregate lowering since that
465-
// splits up copy_addr.
466-
P.addCopyForwarding();
467-
468462
// This DCE pass is the only DCE on ownership SIL. It can cleanup OSSA related
469463
// dead code, e.g. left behind by the ObjCBridgingOptimization.
470464
P.addDCE();

lib/SILOptimizer/Transforms/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ target_sources(swiftSILOptimizer PRIVATE
99
COWOpts.cpp
1010
CSE.cpp
1111
ConditionForwarding.cpp
12-
CopyForwarding.cpp
1312
CopyPropagation.cpp
1413
DeadCodeElimination.cpp
1514
DeadObjectElimination.cpp

0 commit comments

Comments
 (0)