Skip to content

SILCombine: Fix update of substitution map when substituting opened opaque archetypes #31349

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 2 commits into from
Apr 28, 2020
Merged

SILCombine: Fix update of substitution map when substituting opened opaque archetypes #31349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c80776b
Update code to print conformances if SILPrintGenericSpecializationInf…
aschwaighofer Apr 27, 2020
b5f5495
SILCombine: Fix update of substitution map when substituting opened o…
aschwaighofer Apr 27, 2020
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
2 changes: 2 additions & 0 deletions include/swift/AST/ProtocolConformanceRef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ class ProtocolConformanceRef {
void dump(llvm::raw_ostream &out, unsigned indent = 0,
bool details = true) const;

void print(llvm::raw_ostream &out) const;

bool operator==(ProtocolConformanceRef other) const {
return Union == other.Union;
}
Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/Type.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ enum class SubstFlags {
DesugarMemberTypes = 0x02,
/// Substitute types involving opaque type archetypes.
SubstituteOpaqueArchetypes = 0x04,
/// Force substitution of opened archetypes. Normally -- without these flag --
/// opened archetype conformances are not substituted.
ForceSubstituteOpenedExistentials = 0x08,
};

/// Options for performing substitutions into a type.
Expand Down
7 changes: 7 additions & 0 deletions lib/AST/ASTDumper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3330,6 +3330,13 @@ void ProtocolConformanceRef::dump(llvm::raw_ostream &out, unsigned indent,

dumpProtocolConformanceRefRec(*this, out, indent, visited);
}

void ProtocolConformanceRef::print(llvm::raw_ostream &out) const {
llvm::SmallPtrSet<const ProtocolConformance *, 8> visited;
dumpProtocolConformanceRefRec(*this, out, 0, visited);

}

void ProtocolConformance::dump() const {
auto &out = llvm::errs();
dump(out);
Expand Down
3 changes: 2 additions & 1 deletion lib/AST/ProtocolConformance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ ProtocolConformanceRef::subst(Type origType,

// Opened existentials trivially conform and do not need to go through
// substitution map lookup.
if (substType->isOpenedExistential())
if (substType->isOpenedExistential() &&
!options.contains(SubstFlags::ForceSubstituteOpenedExistentials))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we need the flag at all. What breaks if you just remove this check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I ask is that I'm worried there are other places where we missed passing the flag (and it's hard to discover this via grep).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wondered this too and started a run.

It failed in the eager specializer with an non-descriptive assert:

Assertion failed: (!isInvalid()), function getRequirement, file /Users/arnold/Stash4/swift/lib/AST/ProtocolConformance.cpp, line 79.        
Stack dump:
3.      While evaluating request ExecuteSILPipelineRequest(Run pipelines { EarlyModulePasses, HighLevel+EarlyLoopOpt, MidModulePasses+StackPromote, MidLevel } on SIL for Swift.Swift)                                                                                                   
4.      While running pass #49681 SILModuleTransform "EagerSpecializer".                                                                    
0  swift                    0x00000001121a78d5 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37                                          
1  swift                    0x00000001121a6b25 llvm::sys::RunSignalHandlers() + 85                                                          
2  swift                    0x00000001121a7ea6 SignalHandler(int) + 262                                                                     
3  libsystem_platform.dylib 0x00007fff711805fd _sigtramp + 29                                                                                                                                                                                                                            
4  libsystem_platform.dylib 000000000000000000 _sigtramp + 18446603338618763808                                                                                                                                                                                                          
5  libsystem_c.dylib        0x00007fff71056808 abort + 120                                                                                                                                                                                                                               
6  libsystem_c.dylib        0x00007fff71055ac6 err + 0                                                                                                                                                                                                                                   
7  swift                    0x00000001126aba51 swift::ProtocolConformanceRef::getRequirement() const (.cold.1) + 33                                                                                                                                                                      
8  swift                    0x000000010f086148 swift::ProtocolConformanceRef::getRequirement() const + 40                                                                                                                                                                                
9  swift                    0x000000010e9a4b9d swift::WitnessMethodInst::create(swift::SILDebugLocation, swift::CanType, swift::ProtocolConformanceRef, swift::SILDeclRef, swift::SILType, swift::SILFunction*, swift::SILOpenedArchetypesState&) + 125                                  
10 swift                    0x000000010e7a39f5 swift::SILCloner<swift::GenericCloner>::visitWitnessMethodInst(swift::WitnessMethodInst*) + 549                                                                                                                                           
11 swift                    0x000000010e7a04cc swift::SILCloner<swift::GenericCloner>::visitBlocksDepthFirst(swift::SILBasicBlock*) + 444   
12 swift                    0x000000010e79dc99 swift::SILCloner<swift::GenericCloner>::cloneFunctionBody(swift::SILFunction*, swift::SILBasicBlock*, llvm::ArrayRef<swift::SILValue>, bool) + 537                                                                                        
13 swift                    0x000000010e79d7ca swift::GenericCloner::populateCloned() + 1162                                                
14 swift                    0x000000010e7c389e swift::GenericCloner::cloneFunction(swift::SILOptFunctionBuilder&, swift::SILFunction*, swift::ReabstractionInfo const&, swift::SubstitutionMap, llvm::StringRef, std::__1::function<void (swift::SILInstruction*, swift::SILInstruction*)>) + 190                                                                                                                                    
15 swift                    0x000000010e7c35dd swift::GenericFuncSpecializer::tryCreateSpecialization() + 317                               
16 swift                    0x000000010e4bac3e (anonymous namespace)::EagerSpecializerTransform::run() + 1902                                                                                                                                                                            
17 swift                    0x000000010e5ad7ae swift::SILPassManager::runModulePass(unsigned int) + 558                                     
18 swift                    0x000000010e5ae10a swift::SILPassManager::execute() + 666                                                       
19 swift                    0x000000010e5aa8e8 swift::SILPassManager::executePassPipelinePlan(swift::SILPassPipelinePlan const&) + 72       
20 swift                    0x000000010e5aa883 swift::ExecuteSILPipelineRequest::evaluate(swift::Evaluator&, swift::SILPipelineExecutionDescriptor) const + 51                                                                                                                           
21 swift                    0x000000010e5c19bd swift::SimpleRequest<swift::ExecuteSILPipelineRequest, std::__1::tuple<> (swift::SILPipelineExecutionDescriptor), (swift::RequestFlags)1>::evaluateRequest(swift::ExecuteSILPipelineRequest const&, swift::Evaluator&) + 29               
22 swift                    0x000000010e5b3a0e llvm::Expected<swift::ExecuteSILPipelineRequest::OutputType> swift::Evaluator::getResultUncached<swift::ExecuteSILPipelineRequest>(swift::ExecuteSILPipelineRequest const&) + 366                                                         
23 swift                    0x000000010e5aaa24 swift::executePassPipelinePlan(swift::SILModule*, swift::SILPassPipelinePlan const&, bool, swift::irgen::IRGenModule*) + 68                                                                                                               
24 swift                    0x000000010e5b6db7 swift::runSILOptimizationPasses(swift::SILModule&) + 151                                     
25 swift                    0x000000010e04844b swift::CompilerInstance::performSILProcessing(swift::SILModule*) + 619                       
26 swift                    0x000000010df617ac performCompileStepsPostSILGen(swift::CompilerInstance&, swift::CompilerInvocation const&, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> >, llvm::PointerUnion<swift::ModuleDecl*, swift::SourceFile*>, swift::PrimarySpecificPaths const&, int&, swift::FrontendObserver*) + 892                                                                                                                                                                                                               
27 swift                    0x000000010df55bee performCompile(swift::CompilerInstance&, swift::CompilerInvocation const&, llvm::ArrayRef<char const*>, int&, swift::FrontendObserver*) + 7902                                                                                            
28 swift                    0x000000010df52d53 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 3555

Don't know more yet.

return *this;

auto *proto = getRequirement();
Expand Down
11 changes: 9 additions & 2 deletions lib/SIL/IR/SILPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ static void printGenericSpecializationInfo(
raw_ostream &OS, StringRef Kind, StringRef Name,
const GenericSpecializationInformation *SpecializationInfo,
SubstitutionMap Subs = { }) {
if (!SpecializationInfo)
if (!SpecializationInfo && Subs.empty())
return;

auto PrintSubstitutions = [&](SubstitutionMap Subs) {
Expand All @@ -384,6 +384,11 @@ static void printGenericSpecializationInfo(
[&](Type type) { OS << type; },
[&] { OS << ", "; });
OS << '>';
OS << " conformances <";
interleave(Subs.getConformances(),
[&](ProtocolConformanceRef conf) { conf.print(OS); },
[&] { OS << ", ";});
OS << '>';
};

OS << "// Generic specialization information for " << Kind << " " << Name;
Expand Down Expand Up @@ -716,7 +721,9 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
Ctx.printInstructionCallBack(&I);
if (SILPrintGenericSpecializationInfo) {
if (auto AI = ApplySite::isa(const_cast<SILInstruction *>(&I)))
if (AI.getSpecializationInfo() && AI.getCalleeFunction())
if ((AI.getSpecializationInfo() ||
!AI.getSubstitutionMap().empty()) &&
AI.getCalleeFunction())
printGenericSpecializationInfo(
PrintState.OS, "call-site", AI.getCalleeFunction()->getName(),
AI.getSpecializationInfo(), AI.getSubstitutionMap());
Expand Down
3 changes: 2 additions & 1 deletion lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,8 @@ SILInstruction *SILCombiner::createApplyWithConcreteType(
return CEI.lookupExistentialConformance(proto);
}
return ProtocolConformanceRef(proto);
});
},
SubstFlags::ForceSubstituteOpenedExistentials);
}

// We need to make sure that we can a) update Apply to use the new args and b)
Expand Down
91 changes: 90 additions & 1 deletion test/SILOptimizer/sil_combine_concrete_existential.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %target-sil-opt -enable-objc-interop -enable-sil-verify-all %s -sil-combine | %FileCheck %s
// RUN: %target-sil-opt -enable-objc-interop -enable-sil-verify-all %s -sil-combine -sil-print-generic-specialization-info | %FileCheck %s
// RUN: %target-swift-frontend -O %s -emit-ir

// These tests exercise the same SILCombine optimization as
// existential_type_propagation.sil, but cover additional corner
Expand Down Expand Up @@ -615,3 +616,91 @@ bb0(%0 : $*τ_0_0, %1 : $*τ_0_2, %2 : $*τ_0_1):
dealloc_stack %5 : $*P1
return %25 : $()
}


public class MyObject {
deinit
init()
}

public protocol SubscriptionViewControllerDelegate { }

public class SubscriptionViewController {
deinit
init()
}

public protocol ResourceKitProtocol : MyObject { }

public protocol ResourceKitDelegate {
func subscriptionViewController(for resourceKit: ResourceKitProtocol)
}

class ViewController : ResourceKitDelegate {
func subscriptionViewController(for resourceKit: ResourceKitProtocol)
deinit
init()
}

class SubscriptionViewControllerBuilder {
@_hasStorage final let delegate: SubscriptionViewControllerDelegate { get }
init(delegate: SubscriptionViewControllerDelegate)
deinit
}

extension MyObject : SubscriptionViewControllerDelegate { }

// Make sure that we correctly update the substitution map of the apply.
// To satisfy ``τ_0_0 : SubscriptionViewControllerDelegate`` the updated
// substitution map of
// apply %10<$@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol>
// needs to contain the concrete conformance
// ``(normal_conformance type=NSObject protocol=SubscriptionViewControllerDelegate)``
// not ``(abstract_conformance protocol=SubscriptionViewControllerDelegate)``

sil @callee2 : $@convention(thin) <τ_0_0 where τ_0_0 : SubscriptionViewControllerDelegate> (@in τ_0_0, @thick SubscriptionViewControllerBuilder.Type) -> @owned SubscriptionViewControllerBuilder

// CHECK: sil @test_opend_archeype_concrete_conformance_substitution : $@convention(method) (@guaranteed ResourceKitProtocol, @guaranteed ViewController) -> () {
// CHECK: bb0([[ARG:%.*]] : $ResourceKitProtocol, [[ARG2:%.*]] : $ViewController):
// CHECK: [[T1:%.*]] = metatype $@thick SubscriptionViewControllerBuilder.Type
// CHECK: [[T2:%.*]] = open_existential_ref [[ARG]] : $ResourceKitProtocol to $@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol
// CHECK: [[T3:%.*]] = alloc_stack $SubscriptionViewControllerDelegate
// CHECK: [[T4:%.*]] = init_existential_addr [[T3]] : $*SubscriptionViewControllerDelegate, $@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol
// CHECK: store [[T2]] to [[T4]] : $*@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol
// CHECK: [[T5:%.*]] = function_ref @callee2 : $@convention(thin) <τ_0_0 where τ_0_0 : SubscriptionViewControllerDelegate> (@in τ_0_0, @thick SubscriptionViewControllerBuilder.Type) -> @owned SubscriptionViewControllerBuilder
// CHECK: [[T6:%.*]] = alloc_stack $@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol
// CHECK: copy_addr [[T4]] to [initialization] [[T6]] : $*@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol
// CHECK: Generic specialization information for call-site callee2 <ResourceKitProtocol> conformances <(inherited_conformance type=ResourceKitProtocol protocol=SubscriptionViewControllerDelegate
// CHECK: (normal_conformance type=MyObject protocol=SubscriptionViewControllerDelegate))>:
// CHECK: apply [[T5]]<@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol>([[T6]], [[T1]])

sil @test_opend_archeype_concrete_conformance_substitution : $@convention(method) (@guaranteed ResourceKitProtocol, @guaranteed ViewController) -> () {
bb0(%0 : $ResourceKitProtocol, %1 : $ViewController):
%4 = metatype $@thick SubscriptionViewControllerBuilder.Type
%5 = open_existential_ref %0 : $ResourceKitProtocol to $@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol
%6 = alloc_stack $SubscriptionViewControllerDelegate
%7 = init_existential_addr %6 : $*SubscriptionViewControllerDelegate, $@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol
strong_retain %5 : $@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol
store %5 to %7 : $*@opened("E4D92D2A-8893-11EA-9C89-ACDE48001122") ResourceKitProtocol
%10 = function_ref @callee2: $@convention(thin) <τ_0_0 where τ_0_0 : SubscriptionViewControllerDelegate> (@in τ_0_0, @thick SubscriptionViewControllerBuilder.Type) -> @owned SubscriptionViewControllerBuilder
%11 = open_existential_addr mutable_access %6 : $*SubscriptionViewControllerDelegate to $*@opened("FAA82796-8893-11EA-9C89-ACDE48001122") SubscriptionViewControllerDelegate
%12 = alloc_stack $@opened("FAA82796-8893-11EA-9C89-ACDE48001122") SubscriptionViewControllerDelegate
copy_addr %11 to [initialization] %12 : $*@opened("FAA82796-8893-11EA-9C89-ACDE48001122") SubscriptionViewControllerDelegate
%14 = apply %10<@opened("FAA82796-8893-11EA-9C89-ACDE48001122") SubscriptionViewControllerDelegate>(%12, %4) : $@convention(thin) <τ_0_0 where τ_0_0 : SubscriptionViewControllerDelegate> (@in τ_0_0, @thick SubscriptionViewControllerBuilder.Type) -> @owned SubscriptionViewControllerBuilder
destroy_addr %6 : $*SubscriptionViewControllerDelegate
dealloc_stack %12 : $*@opened("FAA82796-8893-11EA-9C89-ACDE48001122") SubscriptionViewControllerDelegate
dealloc_stack %6 : $*SubscriptionViewControllerDelegate
strong_release %14 : $SubscriptionViewControllerBuilder
%19 = tuple ()
return %19 : $()
}

sil_vtable SubscriptionViewControllerBuilder {}
sil_vtable SubscriptionViewController {}
sil_vtable ViewController {}
sil_vtable CCCC {}
sil_vtable CC {}
sil_vtable C {}
sil_vtable C_PQ {}
sil_vtable CDefaultStatic {}
sil_vtable MyObject {}