Skip to content

Commit 4d2c342

Browse files
committed
Update how createApplyWithConcreteType checks if it can update Apply
args createApplyWithConcreteType used to update Apply unconditionally. There may have been cases prior to supporting store instructions that miscompiled because of this but, there are certainly cases after supporting store instructions. Therefore, it is important that the apply is updated only when the substitution can accept the new args.
1 parent ac4fa33 commit 4d2c342

File tree

6 files changed

+17
-37
lines changed

6 files changed

+17
-37
lines changed

lib/SILOptimizer/FunctionSignatureTransforms/ExistentialSpecializer.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,9 +336,6 @@ void ExistentialSpecializer::specializeExistentialArgsInAppliesWithinFunction(
336336
/// Update statistics on the number of functions specialized.
337337
++NumFunctionsWithExistentialArgsSpecialized;
338338

339-
llvm::errs() << "Adding function to pass manger in existential\n";
340-
llvm::errs() << ET.getExistentialSpecializedFunction()->getName() << "\n";
341-
342339
/// Make sure the PM knows about the new specialized inner function.
343340
addFunctionToPassManagerWorklist(ET.getExistentialSpecializedFunction(),
344341
Callee);

lib/SILOptimizer/PassManager/PassManager.cpp

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -407,24 +407,9 @@ void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) {
407407
Mod->registerDeleteNotificationHandler(SFT);
408408
if (breakBeforeRunning(F->getName(), SFT))
409409
LLVM_BUILTIN_DEBUGTRAP;
410-
411-
412-
llvm::errs() << "FUNC NAME: ";
413-
F->printName(llvm::errs());
414-
llvm::errs() << "\nDEMA NAME: " << Demangle::demangleSymbolAsString(F->getName()) << "\n";
415-
llvm::errs() << "PASS NAME: " << SFT->getID() << "\n";
416-
417410
SFT->run();
418411
assert(analysesUnlocked() && "Expected all analyses to be unlocked!");
419412
Mod->removeDeleteNotificationHandler(SFT);
420-
421-
llvm::errs() << "Finished running pass\n\t\t****\n";
422-
if (F->getName().contains("ss6UInt32VSjsSj9magnitude9MagnitudeQzvgTW") ||
423-
F->getName().contains("ss2geoiySbx_q_q0_q1_q2_q3_t_x_q_q0_q1_q2_q3_ttSLRzSLR_SLR0_SLR1_SLR2_SLR3_r4_lF")) {
424-
llvm::errs() << "Dumping module to file...";
425-
F->getModule().dump("/Users/zoe/Developer/swift-source/build/buildbot_incremental/swift-macosx-x86_64/module_dump.txt");
426-
llvm::errs() << "done.\n";
427-
}
428413

429414
auto Delta = (std::chrono::system_clock::now() - StartTime).count();
430415
if (SILPrintPassTime) {

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -893,17 +893,12 @@ SILInstruction *SILCombiner::createApplyWithConcreteType(
893893
// Ensure that we have a concrete value to propagate.
894894
assert(CEI.ConcreteValue);
895895

896-
// If the parameter is expecting a pointer, then we need to create a
897-
// alloc_stack to store the temporary value.
898-
if (Apply.getArgument(ArgIdx)->getType().isAddress()) {
899-
auto argSub =
900-
ConcreteArgumentCopy::generate(CEI, Apply, ArgIdx, BuilderCtx);
901-
if (argSub) {
902-
concreteArgCopies.push_back(*argSub);
903-
NewArgs.push_back(argSub->tempArg);
904-
}
896+
auto argSub =
897+
ConcreteArgumentCopy::generate(CEI, Apply, ArgIdx, BuilderCtx);
898+
if (argSub) {
899+
concreteArgCopies.push_back(*argSub);
900+
NewArgs.push_back(argSub->tempArg);
905901
} else {
906-
// Otherwise, we can just use the value itself.
907902
NewArgs.push_back(CEI.ConcreteValue);
908903
}
909904

@@ -928,7 +923,17 @@ SILInstruction *SILCombiner::createApplyWithConcreteType(
928923
});
929924
}
930925

931-
if (NewArgs.size() != Apply.getNumArguments()) {
926+
bool canUpdateArgs = [&]() {
927+
auto substTy = Apply.getCallee()->getType().substGenericArgs(Apply.getModule(), NewCallSubs, Apply.getFunction()->getTypeExpansionContext()).getAs<SILFunctionType>();
928+
SILFunctionConventions conv(substTy, SILModuleConventions(Apply.getModule()));
929+
bool canUpdate = true;
930+
for (unsigned index = 0; index < conv.getNumSILArguments(); ++index) {
931+
canUpdate &= conv.getSILArgumentType(index) == NewArgs[index]->getType();
932+
}
933+
return canUpdate;
934+
}();
935+
936+
if (!canUpdateArgs) {
932937
// Remove any new instructions created while attempting to optimize this
933938
// apply. Since the apply was never rewritten, if they aren't removed here,
934939
// they will be removed later as dead when visited by SILCombine, causing

lib/SILOptimizer/Transforms/AllocBoxToStack.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -793,8 +793,6 @@ specializePartialApply(SILOptFunctionBuilder &FuncBuilder,
793793
ClonedName);
794794
Cloner.populateCloned();
795795
ClonedFn = Cloner.getCloned();
796-
llvm::errs() << "Adding function to pass manger in alloc box\n";
797-
llvm::errs() << ClonedFn->getName() << "\n";
798796
pass.T->addFunctionToPassManagerWorklist(ClonedFn, F);
799797
}
800798

lib/SILOptimizer/Transforms/Devirtualizer.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,8 @@ bool Devirtualizer::devirtualizeAppliesInFunction(SILFunction &F,
102102
// We may not have optimized these functions yet, and it could
103103
// be beneficial to rerun some earlier passes on the current
104104
// function now that we've made these direct references visible.
105-
if (CalleeFn->isDefinition() && CalleeFn->shouldOptimize()) {
106-
llvm::errs() << "Adding function to pass manger in devirt\n";
107-
llvm::errs() << CalleeFn->getName() << "\n";
105+
if (CalleeFn->isDefinition() && CalleeFn->shouldOptimize())
108106
addFunctionToPassManagerWorklist(CalleeFn, nullptr);
109-
}
110107
}
111108

112109
return Changed;

lib/SILOptimizer/Transforms/GenericSpecializer.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,6 @@ bool GenericSpecializer::specializeAppliesInFunction(SILFunction &F) {
129129
// (as opposed to returning a previous specialization), we need to notify
130130
// the pass manager so that the new functions get optimized.
131131
for (SILFunction *NewF : reverse(NewFunctions)) {
132-
llvm::errs() << "Adding function to pass manger in generic spec\n";
133-
llvm::errs() << NewF->getName() << "\n";
134132
addFunctionToPassManagerWorklist(NewF, Callee);
135133
}
136134
}

0 commit comments

Comments
 (0)