Skip to content

Optimizer: remove the ArrayElementPropagation optimization #77806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ struct AliasAnalysis {
if let apply = inst as? FullApplySite {
// Workaround for quadratic complexity in ARCSequenceOpts.
// We need to use an ever lower budget to not get into noticeable compile time troubles.
let effect = aa.getOwnershipEffect(of: apply, for: obj, path: path)
let effect = aa.getOwnershipEffect(of: apply, for: obj, path: path, complexityBudget: budget / 10)
return effect.destroy
}
return obj.at(path).isEscaping(using: EscapesToInstructionVisitor(target: inst, isAddress: false),
Expand Down Expand Up @@ -421,10 +421,10 @@ struct AliasAnalysis {
}

private func getOwnershipEffect(of apply: FullApplySite, for value: Value,
path: SmallProjectionPath) -> SideEffects.Ownership {
path: SmallProjectionPath,
complexityBudget: Int) -> SideEffects.Ownership {
let visitor = FullApplyEffectsVisitor(apply: apply, calleeAnalysis: context.calleeAnalysis, isAddress: false)
let budget = getComplexityBudget(for: apply.parentFunction)
if let result = value.at(path).visit(using: visitor, complexityBudget: budget, context) {
if let result = value.at(path).visit(using: visitor, complexityBudget: complexityBudget, context) {
// The resulting effects are the argument effects to which `value` escapes to.
return result.ownership
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ private func createOutlinedGlobal(
let elementType = allocVectorBuiltin.substitutionMap.replacementTypes[0]!
let outlinedGlobal = context.createGlobalVariable(
name: context.mangleOutlinedVariable(from: allocVectorBuiltin.parentFunction),
type: elementType, isPrivate: true)
type: elementType, linkage: .private, isLet: false)

let globalBuilder = Builder(staticInitializerOf: outlinedGlobal, context)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@ private func optimizeObjectAllocation(allocRef: AllocRefInstBase, _ context: Fun

let outlinedGlobal = context.createGlobalVariable(
name: context.mangleOutlinedVariable(from: allocRef.parentFunction),
type: allocRef.type, isPrivate: true)
type: allocRef.type, linkage: .private,
// Only if it's a COW object we can be sure that the object allocated in the global is not mutated.
// If someone wants to mutate it, it has to be copied first.
isLet: endOfInitInst is EndCOWMutationInst)

constructObject(of: allocRef, inInitializerOf: outlinedGlobal, storesToClassFields, storesToTailElements, context)
context.erase(instructions: storesToClassFields)
Expand Down Expand Up @@ -562,7 +565,7 @@ private func replace(findStringCall: ApplyInst,

// Create an "opaque" global variable which is passed as inout to
// _findStringSwitchCaseWithCache and into which the function stores the "cache".
let cacheVar = context.createGlobalVariable(name: name, type: cacheType, isPrivate: true)
let cacheVar = context.createGlobalVariable(name: name, type: cacheType, linkage: .private, isLet: false)

let varBuilder = Builder(staticInitializerOf: cacheVar, context)
let zero = varBuilder.createIntegerLiteral(0, type: wordTy)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,34 @@ private func getGlobalInitValue(address: Value, _ context: SimplifyContext) -> V
}
case let bai as BeginAccessInst:
return getGlobalInitValue(address: bai.address, context)
case let rta as RefTailAddrInst:
return getGlobalTailElement(of: rta, index: 0)
case let ia as IndexAddrInst:
if let rta = ia.base as? RefTailAddrInst,
let literal = ia.index as? IntegerLiteralInst,
let index = literal.value
{
return getGlobalTailElement(of: rta, index: index)
}
case let rea as RefElementAddrInst:
if let object = rea.instance.immutableGlobalObjectRoot {
return object.baseOperands[rea.fieldIndex].value
}
default:
break
}
return nil
}

private func getGlobalTailElement(of refTailAddr: RefTailAddrInst, index: Int) -> Value? {
if let object = refTailAddr.instance.immutableGlobalObjectRoot,
index >= 0 && index < object.tailOperands.count
{
return object.tailOperands[index].value
}
return nil
}

private func getInitializerFromInitFunction(of globalAddr: GlobalAddrInst, _ context: SimplifyContext) -> Value? {
guard let dependentOn = globalAddr.dependencyToken,
let builtinOnce = dependentOn as? BuiltinInst,
Expand Down Expand Up @@ -321,6 +343,19 @@ private extension Value {
}
return (baseAddress: self, offset: 0)
}

// If the reference-root of self references a global object, returns the `object` instruction of the
// global's initializer. But only if the global is a let-global.
var immutableGlobalObjectRoot: ObjectInst? {
if let gv = self.referenceRoot as? GlobalValueInst,
gv.global.isLet,
let initval = gv.global.staticInitValue,
let object = initval as? ObjectInst
{
return object
}
return nil
}
}

private extension Instruction {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,9 +391,9 @@ struct FunctionPassContext : MutatingContext {
}
}

func createGlobalVariable(name: String, type: Type, isPrivate: Bool) -> GlobalVariable {
func createGlobalVariable(name: String, type: Type, linkage: Linkage, isLet: Bool) -> GlobalVariable {
let gv = name._withBridgedStringRef {
_bridged.createGlobalVariable($0, type.bridged, isPrivate)
_bridged.createGlobalVariable($0, type.bridged, linkage.bridged, isLet)
}
return gv.globalVar
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ extension ValueUseDefWalker {
return walkUp(value: oer.existential, path: path.push(.existential, index: 0))
case is BeginBorrowInst, is CopyValueInst, is MoveValueInst,
is UpcastInst, is EndCOWMutationInst, is EndInitLetRefInst,
is BeginDeallocRefInst,
is BeginDeallocRefInst, is MarkDependenceInst,
is RefToBridgeObjectInst, is BridgeObjectToRefInst, is MarkUnresolvedNonCopyableValueInst:
return walkUp(value: (def as! Instruction).operands[0].value, path: path)
case let urc as UncheckedRefCastInst:
Expand Down
2 changes: 1 addition & 1 deletion include/swift/SILOptimizer/OptimizerBridging.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ struct BridgedPassContext {
BridgedFunction applySiteCallee) const;

SWIFT_IMPORT_UNSAFE BridgedGlobalVar createGlobalVariable(BridgedStringRef name, BridgedType type,
bool isPrivate) const;
BridgedLinkage linkage, bool isLet) const;
void inlineFunction(BridgedInstruction apply, bool mandatoryInline) const;
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedValue getSILUndef(BridgedType type) const;
BRIDGED_INLINE bool optimizeMemoryAccesses(BridgedFunction f) const;
Expand Down
2 changes: 0 additions & 2 deletions include/swift/SILOptimizer/PassManager/Passes.def
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ IRGEN_PASS(AllocStackHoisting, "alloc-stack-hoisting",
"SIL alloc_stack Hoisting")
PASS(ArrayCountPropagation, "array-count-propagation",
"Array Count Propagation")
PASS(ArrayElementPropagation, "array-element-propagation",
"Array Element Propagation")
SWIFT_FUNCTION_PASS(AssumeSingleThreaded, "sil-assume-single-threaded",
"Assume Single-Threaded Environment")
SWIFT_MODULE_PASS(AsyncDemotion, "async-demotion",
Expand Down
7 changes: 5 additions & 2 deletions lib/IRGen/GenDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2755,8 +2755,11 @@ Address IRGenModule::getAddrOfSILGlobalVariable(SILGlobalVariable *var,
if (forDefinition && !gvar->hasInitializer()) {
if (initVal) {
gvar->setInitializer(initVal);
if (var->isLet() ||
(var->isInitializedObject() && canMakeStaticObjectReadOnly(var->getLoweredType()))) {
if (var->isLet() &&
// Even if it's a `let`, we cannot allocate an object as constant, because it's header
// (metadata, ref-count) is initialized at runtime.
// Exception: if it's an array for which we can initialize the header statically.
(!var->isInitializedObject() || canMakeStaticObjectReadOnly(var->getLoweredType()))) {
gvar->setConstant(true);
}
} else {
Expand Down
11 changes: 7 additions & 4 deletions lib/SILOptimizer/PassManager/PassManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1879,11 +1879,14 @@ BridgedOwnedString BridgedPassContext::mangleWithClosureArgs(
return BridgedOwnedString(mangler.mangle());
}

BridgedGlobalVar BridgedPassContext::createGlobalVariable(BridgedStringRef name, BridgedType type, bool isPrivate) const {
return {SILGlobalVariable::create(
BridgedGlobalVar BridgedPassContext::createGlobalVariable(BridgedStringRef name, BridgedType type, BridgedLinkage linkage, bool isLet) const {
auto *global = SILGlobalVariable::create(
*invocation->getPassManager()->getModule(),
isPrivate ? SILLinkage::Private : SILLinkage::Public, IsNotSerialized,
name.unbridged(), type.unbridged())};
(swift::SILLinkage)linkage, IsNotSerialized,
name.unbridged(), type.unbridged());
if (isLet)
global->setLet(true);
return {global};
}

void BridgedPassContext::fixStackNesting(BridgedFunction function) const {
Expand Down
7 changes: 1 addition & 6 deletions lib/SILOptimizer/PassManager/PassPipeline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,6 @@ void addHighLevelLoopOptPasses(SILPassPipelinePlan &P) {
P.addSimplifyCFG();
P.addPerformanceConstantPropagation();
P.addSimplifyCFG();
P.addArrayElementPropagation();
// End of unrolling passes.
P.addABCOpt();
// Cleanup.
Expand Down Expand Up @@ -467,14 +466,10 @@ void addFunctionPasses(SILPassPipelinePlan &P,

addSimplifyCFGSILCombinePasses(P);

P.addArrayElementPropagation();

// Perform a round of loop/array optimization in the mid-level pipeline after
// potentially inlining semantic calls, e.g. Array append. The high level
// pipeline only optimizes semantic calls *after* inlining (see
// addHighLevelLoopOptPasses). For example, the high-level pipeline may
// perform ArrayElementPropagation and after inlining a level of semantic
// calls, the mid-level pipeline may handle uniqueness hoisting. Do this as
// addHighLevelLoopOptPasses). Do this as
// late as possible before inlining because it must run between runs of the
// inliner when the pipeline restarts.
if (OpLevel == OptimizationLevelKind::MidLevel) {
Expand Down
Loading