Skip to content

Commit c12a25d

Browse files
authored
Merge pull request #16046 from vedantk/cov-visit-extensions
2 parents f45a9c8 + e405fc2 commit c12a25d

File tree

4 files changed

+55
-39
lines changed

4 files changed

+55
-39
lines changed

lib/SIL/SILProfiler.cpp

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,8 @@ static bool canCreateProfilerForAST(ASTNode N) {
9393
assert(hasASTBeenTypeChecked(N) && "Cannot use this AST for profiling");
9494

9595
if (auto *D = N.dyn_cast<Decl *>()) {
96-
// Any mapped function may be profiled. There's an exception for
97-
// constructors because all of the constructors for a type share a single
98-
// profiler.
9996
if (auto *AFD = dyn_cast<AbstractFunctionDecl>(D))
100-
return !isa<ConstructorDecl>(AFD);
97+
return true;
10198

10299
if (isa<TopLevelCodeDecl>(D))
103100
return true;
@@ -798,7 +795,7 @@ struct CoverageMapping : public ASTWalker {
798795
return visitFunctionDecl(*this, AFD, [&] {
799796
CounterExpr &funcCounter = assignCounter(AFD->getBody());
800797

801-
if (isa<ConstructorDecl>(AFD))
798+
if (ParentNominalType && isa<ConstructorDecl>(AFD))
802799
addToCounter(ParentNominalType, funcCounter);
803800
});
804801
} else if (auto *TLCD = dyn_cast<TopLevelCodeDecl>(D)) {
@@ -995,16 +992,6 @@ static StringRef getCurrentFileName(ASTNode Root) {
995992
return {};
996993
}
997994

998-
static void walkTopLevelNodeForProfiling(ASTNode Root, ASTWalker &Walker) {
999-
Root.walk(Walker);
1000-
1001-
// Visit extensions when walking through a nominal type.
1002-
auto *NTD = dyn_cast_or_null<NominalTypeDecl>(Root.dyn_cast<Decl *>());
1003-
if (NTD)
1004-
for (ExtensionDecl *ED : NTD->getExtensions())
1005-
ED->walk(Walker);
1006-
}
1007-
1008995
void SILProfiler::assignRegionCounters() {
1009996
const auto &SM = M.getASTContext().SourceMgr;
1010997

@@ -1040,15 +1027,15 @@ void SILProfiler::assignRegionCounters() {
10401027
CurrentFuncName, getEquivalentPGOLinkage(CurrentFuncLinkage),
10411028
CurrentFileName);
10421029

1043-
walkTopLevelNodeForProfiling(Root, Mapper);
1030+
Root.walk(Mapper);
10441031

10451032
NumRegionCounters = Mapper.NextCounter;
10461033
// TODO: Mapper needs to calculate a function hash as it goes.
10471034
PGOFuncHash = 0x0;
10481035

10491036
if (EmitCoverageMapping) {
10501037
CoverageMapping Coverage(SM);
1051-
walkTopLevelNodeForProfiling(Root, Coverage);
1038+
Root.walk(Coverage);
10521039
CovMap =
10531040
Coverage.emitSourceRegions(M, CurrentFuncName, PGOFuncName, PGOFuncHash,
10541041
RegionCounterMap, CurrentFileName);
@@ -1066,7 +1053,7 @@ void SILProfiler::assignRegionCounters() {
10661053
}
10671054
PGOMapping pgoMapper(RegionLoadedCounterMap, LoadedCounts,
10681055
RegionCondToParentMap);
1069-
walkTopLevelNodeForProfiling(Root, pgoMapper);
1056+
Root.walk(pgoMapper);
10701057
}
10711058
}
10721059

lib/SILGen/SILGen.cpp

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -489,13 +489,22 @@ static bool haveProfiledAssociatedFunction(SILDeclRef constant) {
489489
}
490490

491491
SILProfiler *
492-
SILGenModule::getOrCreateProfilerForConstructors(NominalTypeDecl *decl) {
493-
assert(decl && "No nominal type for constructor lookup");
494-
492+
SILGenModule::getOrCreateProfilerForConstructors(DeclContext *ctx,
493+
ConstructorDecl *cd) {
495494
const auto &Opts = M.getOptions();
496495
if (!Opts.GenerateProfile && Opts.UseProfile.empty())
497496
return nullptr;
498497

498+
// Profile nominal types and extensions separately, as they may live in
499+
// distinct files. For extensions, just pass in the constructor, because
500+
// there are no stored property initializers to visit.
501+
Decl *decl = nullptr;
502+
if (ctx->isExtensionContext())
503+
decl = cd;
504+
else
505+
decl = ctx->getAsNominalTypeOrNominalTypeExtensionContext();
506+
assert(decl && "No decl available for profiling in this context");
507+
499508
SILProfiler *&profiler = constructorProfilers[decl];
500509
if (!profiler)
501510
profiler = SILProfiler::create(M, ForDefinition, decl);
@@ -755,7 +764,7 @@ void SILGenModule::emitConstructor(ConstructorDecl *decl) {
755764

756765
bool ForCoverageMapping = doesASTRequireProfiling(M, decl);
757766

758-
if (auto *clsDecl = declCtx->getAsClassOrClassExtensionContext()) {
767+
if (declCtx->getAsClassOrClassExtensionContext()) {
759768
// Class constructors have separate entry points for allocation and
760769
// initialization.
761770
emitOrDelayFunction(
@@ -772,24 +781,24 @@ void SILGenModule::emitConstructor(ConstructorDecl *decl) {
772781
SILDeclRef initConstant(decl, SILDeclRef::Kind::Initializer);
773782
emitOrDelayFunction(
774783
*this, initConstant,
775-
[this, initConstant, decl, clsDecl](SILFunction *initF) {
784+
[this, initConstant, decl, declCtx](SILFunction *initF) {
776785
preEmitFunction(initConstant, decl, initF, decl);
777786
PrettyStackTraceSILFunction X("silgen constructor initializer",
778787
initF);
779-
initF->setProfiler(getOrCreateProfilerForConstructors(clsDecl));
788+
initF->setProfiler(
789+
getOrCreateProfilerForConstructors(declCtx, decl));
780790
SILGenFunction(*this, *initF).emitClassConstructorInitializer(decl);
781791
postEmitFunction(initConstant, initF);
782792
},
783793
/*forceEmission=*/ForCoverageMapping);
784794
}
785795
} else {
786796
// Struct and enum constructors do everything in a single function.
787-
auto *nomDecl = declCtx->getAsNominalTypeOrNominalTypeExtensionContext();
788797
emitOrDelayFunction(
789-
*this, constant, [this, constant, decl, nomDecl](SILFunction *f) {
798+
*this, constant, [this, constant, decl, declCtx](SILFunction *f) {
790799
preEmitFunction(constant, decl, f, decl);
791800
PrettyStackTraceSILFunction X("silgen emitConstructor", f);
792-
f->setProfiler(getOrCreateProfilerForConstructors(nomDecl));
801+
f->setProfiler(getOrCreateProfilerForConstructors(declCtx, decl));
793802
SILGenFunction(*this, *f).emitValueConstructor(decl);
794803
postEmitFunction(constant, f);
795804
});
@@ -997,10 +1006,8 @@ emitStoredPropertyInitialization(PatternBindingDecl *pbd, unsigned i) {
9971006
PrettyStackTraceSILFunction X("silgen emitStoredPropertyInitialization", f);
9981007

9991008
// Inherit a profiler instance from the constructor.
1000-
auto *nominalDecl =
1001-
var->getDeclContext()->getAsNominalTypeOrNominalTypeExtensionContext();
1002-
if (nominalDecl)
1003-
f->setProfiler(getOrCreateProfilerForConstructors(nominalDecl));
1009+
f->setProfiler(
1010+
getOrCreateProfilerForConstructors(var->getDeclContext(), nullptr));
10041011

10051012
SILGenFunction(*this, *f).emitGeneratorFunction(constant, init);
10061013
postEmitFunction(constant, f);

lib/SILGen/SILGen.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
9696
/// The most recent conformance...
9797
NormalProtocolConformance *lastEmittedConformance = nullptr;
9898

99-
/// Profiler instances for constructors, grouped by associated nominal type.
99+
/// Profiler instances for constructors, grouped by associated decl.
100100
/// Each profiler is shared by all member initializers for a nominal type.
101-
llvm::DenseMap<NominalTypeDecl *, SILProfiler *> constructorProfilers;
101+
/// Constructors within extensions are profiled separately.
102+
llvm::DenseMap<Decl *, SILProfiler *> constructorProfilers;
102103

103104
SILFunction *emitTopLevelFunction(SILLocation Loc);
104105

@@ -440,9 +441,12 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
440441
/// Emit a property descriptor for the given storage decl if it needs one.
441442
void tryEmitPropertyDescriptor(AbstractStorageDecl *decl);
442443

443-
/// Get or create the profiler instance for a nominal type's constructors.
444-
SILProfiler *getOrCreateProfilerForConstructors(NominalTypeDecl *decl);
445-
444+
/// Get or create the shared profiler instance for a type's constructors.
445+
/// This takes care to create separate profilers for extensions, which may
446+
/// reside in a different file than the one where the base type is defined.
447+
SILProfiler *getOrCreateProfilerForConstructors(DeclContext *ctx,
448+
ConstructorDecl *cd);
449+
446450
private:
447451
/// Emit the deallocator for a class that uses the objc allocator.
448452
void emitObjCAllocatorDestructor(ClassDecl *cd, DestructorDecl *dd);

test/SILGen/coverage_decls.swift

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,25 @@
1-
// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -profile-generate -profile-coverage-mapping -emit-sorted-sil -emit-sil -module-name coverage_decls -primary-file %s %S/Inputs/coverage_imports.swift | %FileCheck %s
1+
// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -profile-generate -profile-coverage-mapping -emit-sorted-sil -emit-sil -module-name coverage_decls %s %S/Inputs/coverage_imports.swift | %FileCheck %s -check-prefix=ALL
2+
// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -profile-generate -profile-coverage-mapping -emit-sorted-sil -emit-sil -module-name coverage_decls -primary-file %s %S/Inputs/coverage_imports.swift | %FileCheck %s -check-prefix=PRIMARY
23

3-
// CHECK: sil_coverage_map {{.*}} $S14coverage_decls4mainyyF
4-
// CHECK-NOT: sil_coverage_map
4+
// ALL: sil_coverage_map {{.*}} // closure #1 () -> Swift.Int in coverage_decls.Box.x.getter : Swift.Int
5+
// ALL: sil_coverage_map {{.*}} // coverage_decls.Box.init(y: Swift.Int) -> coverage_decls.Box
6+
// ALL: sil_coverage_map {{.*}} // coverage_decls.Box.init(z: Swift.String) -> coverage_decls.Box
7+
// ALL: sil_coverage_map {{.*}} // coverage_decls.main() -> ()
8+
// ALL: sil_coverage_map {{.*}} // __ntd_Box
9+
10+
// PRIMARY-NOT: sil_coverage_map
11+
// PRIMARY: sil_coverage_map {{.*}} // coverage_decls.Box.init(y: Swift.Int) -> coverage_decls.Box
12+
// PRIMARY: sil_coverage_map {{.*}} // coverage_decls.Box.init(z: Swift.String) -> coverage_decls.Box
13+
// PRIMARY: sil_coverage_map {{.*}} // coverage_decls.main() -> ()
14+
// PRIMARY-NOT: sil_coverage_map
15+
16+
extension Box {
17+
init(y: Int) { self.init() }
18+
}
19+
20+
extension Box {
21+
init(z: String) { self.init() }
22+
}
523

624
func main() {
725
var b = Box()

0 commit comments

Comments
 (0)