Skip to content

Commit 1f80bbe

Browse files
committed
[Profiler] Fix up a couple uses of emitProfilerIncrement
Remove a case where we know we don't have a profiler, and avoid incrementing for a distributed factory method. Otherwise such cases could pass a null ASTNode, which will become an assertion failure in a future commit. While we're here, let's standardize on emitting the profiler increment for function entry after the prolog, as if there's e.g an unreachable parameter, the increment can be safely elided anyway.
1 parent 3e8d7da commit 1f80bbe

File tree

4 files changed

+17
-9
lines changed

4 files changed

+17
-9
lines changed

lib/SILGen/SILGenConstructor.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -695,8 +695,6 @@ void SILGenFunction::emitClassConstructorAllocator(ConstructorDecl *ctor) {
695695
SILValue initedSelfValue = emitApplyWithRethrow(Loc, initVal.forward(*this),
696696
initTy, subMap, args);
697697

698-
emitProfilerIncrement(ctor->getTypecheckedBody());
699-
700698
// Return the initialized 'self'.
701699
B.createReturn(ImplicitReturnLocation(Loc), initedSelfValue);
702700
}

lib/SILGen/SILGenFunction.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,6 @@ void SILGenFunction::emitFunction(FuncDecl *fd) {
537537
MagicFunctionName = SILGenModule::getMagicFunctionName(fd);
538538

539539
auto captureInfo = SGM.M.Types.getLoweredLocalCaptures(SILDeclRef(fd));
540-
emitProfilerIncrement(fd->getTypecheckedBody());
541540
emitProlog(captureInfo, fd->getParameters(), fd->getImplicitSelfDecl(), fd,
542541
fd->getResultInterfaceType(), fd->hasThrows(), fd->getThrowsLoc());
543542

@@ -547,7 +546,9 @@ void SILGenFunction::emitFunction(FuncDecl *fd) {
547546
} else {
548547
prepareEpilog(fd->getResultInterfaceType(),
549548
fd->hasThrows(), CleanupLocation(fd));
550-
549+
550+
emitProfilerIncrement(fd->getTypecheckedBody());
551+
551552
// Emit the actual function body as usual
552553
emitStmt(fd->getTypecheckedBody());
553554

@@ -564,12 +565,12 @@ void SILGenFunction::emitClosure(AbstractClosureExpr *ace) {
564565
auto resultIfaceTy = ace->getResultType()->mapTypeOutOfContext();
565566
auto captureInfo = SGM.M.Types.getLoweredLocalCaptures(
566567
SILDeclRef(ace));
567-
emitProfilerIncrement(ace);
568568
emitProlog(captureInfo, ace->getParameters(), /*selfParam=*/nullptr,
569569
ace, resultIfaceTy, ace->isBodyThrowing(), ace->getLoc(),
570570
SGM.M.Types.getConstantAbstractionPattern(SILDeclRef(ace)));
571571
prepareEpilog(resultIfaceTy, ace->isBodyThrowing(), CleanupLocation(ace));
572572

573+
emitProfilerIncrement(ace);
573574
if (auto *ce = dyn_cast<ClosureExpr>(ace)) {
574575
emitStmt(ce->getBody());
575576
} else {
@@ -1170,6 +1171,11 @@ void SILGenFunction::emitProfilerIncrement(ASTNode N) {
11701171
assert(CounterIt != RegionCounterMap.end() &&
11711172
"cannot increment non-existent counter");
11721173

1174+
// If we're at an unreachable point, the increment can be elided as the
1175+
// counter cannot be incremented.
1176+
if (!B.hasValidInsertionPoint())
1177+
return;
1178+
11731179
B.createIncrementProfilerCounter(
11741180
getLocation(N), CounterIt->second, SP->getPGOFuncName(),
11751181
SP->getNumRegionCounters(), SP->getPGOFuncHash());

test/Profiler/coverage_closure_returns_never.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -profile-generate -profile-coverage-mapping -emit-sil -module-name coverage_closure_returns_never %s | %FileCheck %s
22

3+
// We don't need to emit the increment_profiler_counter, as the function is
4+
// uncallable.
35
// CHECK-LABEL: closure #1 (Swift.Never) -> Swift.Never in coverage_closure_returns_never.closure_with_fatal_error(Swift.Array<Swift.Never>) -> ()
4-
// CHECK: increment_profiler_counter 0
5-
// CHECK-NEXT: [[LOAD:%.*]] = load {{.*}} : $*Never
6+
// CHECK-NOT: increment_profiler_counter
7+
// CHECK: [[LOAD:%.*]] = load {{.*}} : $*Never
68
// CHECK-NEXT: debug_value [[LOAD]] : $Never
79
// CHECK-NEXT: unreachable
810

test/Profiler/coverage_non_constructible_enum.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
// Reduced from Carthage (https://github.com/Carthage/Carthage).
44
enum NoError: Error, Equatable {
5+
// We don't need to emit the increment_profiler_counter, as the function is
6+
// uncallable.
57
// CHECK-LABEL: static coverage_non_constructible_enum.NoError.== infix
6-
// CHECK: increment_profiler_counter
7-
// CHECK: unreachable
8+
// CHECK-NOT: increment_profiler_counter
9+
// CHECK: unreachable
810

911
// CHECK-LABEL: sil_coverage_map {{.*}} static coverage_non_constructible_enum.NoError.== infix
1012
// CHECK-NEXT: [[@LINE+1]]:54 -> [[@LINE+3]]:4 : 0

0 commit comments

Comments
 (0)