Skip to content

Fix ExistentialSpecializer to correctly cleanup storage. #25038

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 7 commits into from
Jun 3, 2019
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
3 changes: 0 additions & 3 deletions include/swift/AST/SILOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ class SILOptions {
/// Remove all runtime assertions during optimizations.
bool RemoveRuntimeAsserts = false;

/// Enable existential specializer optimization.
bool ExistentialSpecializer = false;

/// Controls whether the SIL ARC optimizations are run.
bool EnableARCOptimizations = true;

Expand Down
3 changes: 0 additions & 3 deletions include/swift/Option/FrontendOptions.td
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,6 @@ def sil_unroll_threshold : Separate<["-"], "sil-unroll-threshold">,
MetaVarName<"<250>">,
HelpText<"Controls the aggressiveness of loop unrolling">;

def sil_existential_specializer : Flag<["-"], "sil-existential-specializer">,
HelpText<"Enable SIL existential specializer optimization">;

def sil_merge_partial_modules : Flag<["-"], "sil-merge-partial-modules">,
HelpText<"Merge SIL from all partial swiftmodules into the final module">;

Expand Down
3 changes: 0 additions & 3 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -702,9 +702,6 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
return true;
}
}
if (Args.hasArg(OPT_sil_existential_specializer)) {
Opts.ExistentialSpecializer = true;
}
if (const Arg *A = Args.getLastArg(OPT_num_threads)) {
if (StringRef(A->getValue()).getAsInteger(10, Opts.NumThreads)) {
Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@

using namespace swift;

static llvm::cl::opt<bool>
EnableExistentialSpecializer("enable-existential-specializer",
llvm::cl::Hidden,
llvm::cl::init(true));

STATISTIC(NumFunctionsWithExistentialArgsSpecialized,
"Number of functions with existential args specialized");

Expand Down Expand Up @@ -66,7 +71,7 @@ class ExistentialSpecializer : public SILFunctionTransform {
auto *F = getFunction();

/// Don't optimize functions that should not be optimized.
if (!F->shouldOptimize() || !F->getModule().getOptions().ExistentialSpecializer) {
if (!F->shouldOptimize() || !EnableExistentialSpecializer) {
return;
}

Expand Down Expand Up @@ -110,19 +115,6 @@ bool ExistentialSpecializer::findConcreteTypeFromSoleConformingType(
return true;
}

/// Check if the argument Arg is used in a destroy_use instruction.
static void
findIfCalleeUsesArgInDestroyUse(SILValue Arg,
ExistentialTransformArgumentDescriptor &ETAD) {
for (Operand *ArgUse : Arg->getUses()) {
auto *ArgUser = ArgUse->getUser();
if (isa<DestroyAddrInst>(ArgUser)) {
ETAD.DestroyAddrUse = true;
break;
}
}
}

/// Helper function to ensure that the argument is not InOut or InOut_Aliasable
static bool isNonInoutIndirectArgument(SILValue Arg,
SILArgumentConvention ArgConvention) {
Expand Down Expand Up @@ -188,24 +180,27 @@ bool ExistentialSpecializer::canSpecializeExistentialArgsInFunction(
continue;
}

/// Determine attributes of the existential addr arguments such as
/// destroy_use, immutable_access.
/// Determine attributes of the existential addr argument.
ExistentialTransformArgumentDescriptor ETAD;
auto paramInfo = origCalleeConv.getParamInfoForSILArg(Idx);
ETAD.AccessType = (paramInfo.isIndirectMutating() || paramInfo.isConsumed())
// The ExistentialSpecializerCloner copies the incoming generic argument
// into an existential. This won't work if the original argument is
// mutated. Furthermore, SILCombine would not be able to replace a mutated
// existential with a concrete value, so the specialization thunk could not
// be optimized away.
if (paramInfo.isIndirectMutating())
continue;

ETAD.AccessType = paramInfo.isConsumed()
? OpenedExistentialAccess::Mutable
: OpenedExistentialAccess::Immutable;
ETAD.DestroyAddrUse = false;
if ((CalleeArgs[Idx]->getType().getPreferredExistentialRepresentation(
F->getModule()))
!= ExistentialRepresentation::Class)
findIfCalleeUsesArgInDestroyUse(CalleeArg, ETAD);
ETAD.isConsumed = paramInfo.isConsumed();

/// Save the attributes
ExistentialArgDescriptor[Idx] = ETAD;
LLVM_DEBUG(llvm::dbgs()
<< "ExistentialSpecializer Pass:Function: " << F->getName()
<< " Arg:" << Idx << "has a concrete type.\n");
<< " Arg:" << Idx << " has a concrete type.\n");
returnFlag |= true;
}
return returnFlag;
Expand Down
Loading