Skip to content

Revert "Propagate concrete types of arguments to apply" #19248

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.

Sign up for GitHub

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 1 commit into from
Sep 11, 2018
Merged

Revert "Propagate concrete types of arguments to apply" #19248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 3 additions & 7 deletions lib/SILOptimizer/SILCombiner/SILCombiner.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,13 +295,9 @@ class SILCombiner :
private:
FullApplySite rewriteApplyCallee(FullApplySite apply, SILValue callee);

bool canReplaceArg(FullApplySite Apply, const ConcreteExistentialInfo &CEI,
unsigned ArgIdx);
SILInstruction *createApplyWithConcreteType(
FullApplySite Apply,
const llvm::SmallDenseMap<unsigned, const ConcreteExistentialInfo *>
&CEIs,
SILBuilderContext &BuilderCtx);
SILInstruction *createApplyWithConcreteType(FullApplySite Apply,
const ConcreteExistentialInfo &CEI,
SILBuilderContext &BuilderCtx);

// Common utility function to replace the WitnessMethodInst using a
// BuilderCtx.
Expand Down
220 changes: 84 additions & 136 deletions lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,26 +693,22 @@ SILCombiner::propagateSoleConformingType(FullApplySite Apply,
SILBuilderContext BuilderCtx(M, Builder.getTrackingList());
replaceWitnessMethodInst(WMI, BuilderCtx, ConcreteType,
*(CEI.ExistentialSubs.getConformances().begin()));
// Construct the map for Self to be used for createApplyWithConcreteType.
llvm::SmallDenseMap<unsigned, const ConcreteExistentialInfo *> CEIs;
CEIs.insert(std::pair<unsigned, const ConcreteExistentialInfo *>(
Apply.getCalleeArgIndex(Apply.getSelfArgumentOperand()), &CEI));
/// Create the new apply instruction using the concrete type.
auto *NewAI = createApplyWithConcreteType(Apply, CEIs, BuilderCtx);
auto *NewAI = createApplyWithConcreteType(Apply, CEI, BuilderCtx);
return NewAI;
}

/// Given an Apply and an argument value produced by InitExistentialAddrInst,
/// return true if the argument can be replaced by a copy of its value.
///
/// FIXME: remove this helper when we can assume SIL opaque values.
static bool canReplaceCopiedArg(FullApplySite Apply,
static bool canReplaceCopiedSelf(FullApplySite Apply,
SILInstruction *InitExistential,
DominanceAnalysis *DA, unsigned ArgIdx) {
// If the witness method mutates Arg, we cannot replace Arg with
DominanceAnalysis *DA) {
// If the witness method mutates self, we cannot replace self with
// the source of a copy. Otherwise the call would modify another value than
// the original argument.
if (Apply.getOrigCalleeType()->getParameters()[ArgIdx].isIndirectMutating())
// the original self.
if (Apply.getOrigCalleeType()->getSelfParameter().isIndirectMutating())
return false;

auto *DT = DA->get(Apply.getFunction());
Expand Down Expand Up @@ -755,54 +751,6 @@ static bool canReplaceCopiedArg(FullApplySite Apply,
return true;
}

// Check the legal conditions under which a Arg parameter (specified as ArgIdx)
// can be replaced with a concrete type. Concrete type info is passed as CEI
// argument.
bool SILCombiner::canReplaceArg(FullApplySite Apply,
const ConcreteExistentialInfo &CEI,
unsigned ArgIdx) {

// Don't specialize apply instructions that return the callee's Arg type,
// because this optimization does not know how to substitute types in the
// users of this apply. In the function type substitution below, all
// references to OpenedArchetype will be substituted. So walk to type to
// find all possible references, such as returning Optional<Arg>.
if (Apply.getType().getASTType().findIf(
[&CEI](Type t) -> bool { return t->isEqual(CEI.OpenedArchetype); })) {
return false;
}
// Bail out if any other arguments or indirect result that refer to the
// OpenedArchetype. The following optimization substitutes all occurrences
// of OpenedArchetype in the function signature, but will only rewrite the
// Arg operand.
//
// Note that the language does not allow Self to occur in contravariant
// position. However, SIL does allow this and it can happen as a result of
// upstream transformations. Since this is bail-out logic, it must handle
// all verifiable SIL.

// This bailout check is also needed for non-Self arguments [including Self].
unsigned NumApplyArgs = Apply.getNumArguments();
for (unsigned Idx = 0; Idx < NumApplyArgs; Idx++) {
if (Idx == ArgIdx)
continue;
if (Apply.getArgument(Idx)->getType().getASTType().findIf(
[&CEI](Type t) -> bool {
return t->isEqual(CEI.OpenedArchetype);
})) {
return false;
}
}
// The apply can only be rewritten in terms of the concrete value if it is
// legal to pass that value as the Arg argument.
if (CEI.isCopied && (!CEI.InitExistential ||
!canReplaceCopiedArg(Apply, CEI.InitExistential, DA, ArgIdx))) {
return false;
}
// It is safe to replace Arg.
return true;
}

/// Rewrite the given method apply instruction in terms of the provided conrete
/// type information.
///
Expand All @@ -826,61 +774,74 @@ bool SILCombiner::canReplaceArg(FullApplySite Apply,
/// FIXME: Protocol methods (witness or default) that return Self will be given
/// a new return type. This implementation fails to update the type signature of
/// SSA uses in those cases. Currently we bail out on methods that return Self.
SILInstruction *SILCombiner::createApplyWithConcreteType(
FullApplySite Apply,
const llvm::SmallDenseMap<unsigned, const ConcreteExistentialInfo *> &CEIs,
SILBuilderContext &BuilderCtx) {

// Ensure that the callee is polymorphic.
SILInstruction *
SILCombiner::createApplyWithConcreteType(FullApplySite Apply,
const ConcreteExistentialInfo &CEI,
SILBuilderContext &BuilderCtx) {
assert(Apply.getOrigCalleeType()->isPolymorphic());

// Create the new set of arguments to apply including their substitutions.
SubstitutionMap NewCallSubs = Apply.getSubstitutionMap();
SmallVector<SILValue, 8> NewArgs;
unsigned NumApplyArgs = Apply.getNumArguments();
bool UpdatedArgs = false;
for (unsigned ArgIdx = 0; ArgIdx < NumApplyArgs; ArgIdx++) {
auto ArgIt = CEIs.find(ArgIdx);
if (ArgIt == CEIs.end()) {
// Use the old argument if it does not have a valid concrete existential.
NewArgs.push_back(Apply.getArgument(ArgIdx));
continue;
}
auto *CEI = ArgIt->second;
// Check for Arg's concrete type propagation legality.
if (!canReplaceArg(Apply, *CEI, ArgIdx)) {
NewArgs.push_back(Apply.getArgument(ArgIdx));
continue;
// Don't specialize apply instructions that return the callee's Self type,
// because this optimization does not know how to substitute types in the
// users of this apply. In the function type substitution below, all
// references to OpenedArchetype will be substituted. So walk to type to find
// all possible references, such as returning Optional<Self>.
if (Apply.getType().getASTType().findIf(
[&CEI](Type t) -> bool { return t->isEqual(CEI.OpenedArchetype); })) {
return nullptr;
}
// Bail out if any non-self arguments or indirect result that refer to the
// OpenedArchetype. The following optimization substitutes all occurrences of
// OpenedArchetype in the function signature, but will only rewrite the self
// operand.
//
// Note that the language does not allow Self to occur in contravariant
// position. However, SIL does allow this and it can happen as a result of
// upstream transformations. Since this is bail-out logic, it must handle all
// verifiable SIL.
for (auto Arg : Apply.getArgumentsWithoutSelf()) {
if (Arg->getType().getASTType().findIf([&CEI](Type t) -> bool {
return t->isEqual(CEI.OpenedArchetype);
})) {
return nullptr;
}
UpdatedArgs = true;
// Ensure that we have a concrete value to propagate.
assert(CEI->ConcreteValue);
NewArgs.push_back(CEI->ConcreteValue);
// Form a new set of substitutions where the argument is
// replaced with a concrete type.
NewCallSubs = NewCallSubs.subst(
[&](SubstitutableType *type) -> Type {
if (type == CEI->OpenedArchetype)
return CEI->ConcreteType;
return type;
},
[&](CanType origTy, Type substTy,
ProtocolDecl *proto) -> Optional<ProtocolConformanceRef> {
if (origTy->isEqual(CEI->OpenedArchetype)) {
assert(substTy->isEqual(CEI->ConcreteType));
// Do a conformance lookup on this witness requirement using the
// existential's conformances. The witness requirement may be a
// base type of the existential's requirements.
return CEI->lookupExistentialConformance(proto);
}
return ProtocolConformanceRef(proto);
});
}

if (!UpdatedArgs)
// The apply can only be rewritten in terms of the concrete value if it is
// legal to pass that value as the self argument.
if (CEI.isCopied && (!CEI.InitExistential ||
!canReplaceCopiedSelf(Apply, CEI.InitExistential, DA))) {
return nullptr;
}

// Create a set of arguments.
SmallVector<SILValue, 8> NewArgs;
for (auto Arg : Apply.getArgumentsWithoutSelf()) {
NewArgs.push_back(Arg);
}
NewArgs.push_back(CEI.ConcreteValue);

assert(Apply.getOrigCalleeType()->isPolymorphic());

// Form a new set of substitutions where Self is
// replaced by a concrete type.
SubstitutionMap OrigCallSubs = Apply.getSubstitutionMap();
SubstitutionMap NewCallSubs = OrigCallSubs.subst(
[&](SubstitutableType *type) -> Type {
if (type == CEI.OpenedArchetype)
return CEI.ConcreteType;
return type;
},
[&](CanType origTy, Type substTy,
ProtocolDecl *proto) -> Optional<ProtocolConformanceRef> {
if (origTy->isEqual(CEI.OpenedArchetype)) {
assert(substTy->isEqual(CEI.ConcreteType));
// Do a conformance lookup on this witness requirement using the
// existential's conformances. The witness requirement may be a base
// type of the existential's requirements.
return CEI.lookupExistentialConformance(proto).getValue();
}
return ProtocolConformanceRef(proto);
});

// Now create the new apply instruction.
SILBuilderWithScope ApplyBuilder(Apply.getInstruction(), BuilderCtx);
FullApplySite NewApply;
if (auto *TAI = dyn_cast<TryApplyInst>(Apply))
Expand Down Expand Up @@ -959,12 +920,8 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply,
replaceWitnessMethodInst(WMI, BuilderCtx, CEI.ConcreteType,
SelfConformance);
}
// Construct the map for Self to be used for createApplyWithConcreteType.
llvm::SmallDenseMap<unsigned, const ConcreteExistentialInfo *> CEIs;
CEIs.insert(std::pair<unsigned, const ConcreteExistentialInfo *>(
Apply.getCalleeArgIndex(Apply.getSelfArgumentOperand()), &CEI));
// Try to rewrite the apply.
return createApplyWithConcreteType(Apply, CEIs, BuilderCtx);
return createApplyWithConcreteType(Apply, CEI, BuilderCtx);
}

/// Rewrite a protocol extension lookup type from an archetype to a concrete
Expand All @@ -979,36 +936,27 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply,
/// ==> apply %f<C : P>(%ref)
SILInstruction *
SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite Apply) {
// This optimization requires a generic argument.
if (!Apply.hasSubstitutions())
// This optimization requires a generic self argument.
if (!Apply.hasSelfArgument() || !Apply.hasSubstitutions())
return nullptr;

// Try to derive the concrete type of self and a related conformance from
// the found init_existential.
const ConcreteExistentialInfo CEI(Apply.getSelfArgumentOperand());
if (!CEI.isValid())
return nullptr;

SILBuilderContext BuilderCtx(Builder.getModule(), Builder.getTrackingList());
SILOpenedArchetypesTracker OpenedArchetypesTracker(&Builder.getFunction());
BuilderCtx.setOpenedArchetypesTracker(&OpenedArchetypesTracker);
llvm::SmallDenseMap<unsigned, const ConcreteExistentialInfo *> CEIs;
for (unsigned ArgIdx = 0; ArgIdx < Apply.getNumArguments(); ArgIdx++) {
auto ArgASTType = Apply.getArgument(ArgIdx)->getType().getASTType();
if (!ArgASTType->hasArchetype())
continue;
const ConcreteExistentialInfo CEI(Apply.getArgumentOperands()[ArgIdx]);
if (!CEI.isValid())
continue;

CEIs.insert(
std::pair<unsigned, const ConcreteExistentialInfo *>(ArgIdx, &CEI));

if (CEI.ConcreteType->isOpenedExistential()) {
// Temporarily record this opened existential def in this local
// BuilderContext before rewriting the witness method.
OpenedArchetypesTracker.addOpenedArchetypeDef(
cast<ArchetypeType>(CEI.ConcreteType), CEI.ConcreteTypeDef);
}
if (CEI.ConcreteType->isOpenedExistential()) {
// Temporarily record this opened existential def in this local
// BuilderContext before rewriting the witness method.
OpenedArchetypesTracker.addOpenedArchetypeDef(
cast<ArchetypeType>(CEI.ConcreteType), CEI.ConcreteTypeDef);
}
// Bail, if no argument has a concrete existential to propagate.
if (CEIs.empty())
return nullptr;
return createApplyWithConcreteType(Apply, CEIs, BuilderCtx);
// Perform the transformation by rewriting the apply.
return createApplyWithConcreteType(Apply, CEI, BuilderCtx);
}

/// \brief Check that all users of the apply are retain/release ignoring one
Expand Down
Loading