-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[ctxprof] Don't lower instrumentation for noreturn
functions
#134932
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
[ctxprof] Don't lower instrumentation for noreturn
functions
#134932
Conversation
@llvm/pr-subscribers-pgo Author: Mircea Trofin (mtrofin) ChangesFull diff: https://github.com/llvm/llvm-project/pull/134932.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp b/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
index 2f8d7766bb588..a314457423819 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
@@ -8,6 +8,7 @@
//
#include "llvm/Transforms/Instrumentation/PGOCtxProfLowering.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Analysis/CtxProfAnalysis.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/IR/Analysis.h"
@@ -206,6 +207,17 @@ PreservedAnalyses PGOCtxProfLoweringPass::run(Module &M,
bool CtxInstrumentationLowerer::lowerFunction(Function &F) {
if (F.isDeclaration())
return false;
+
+ // Probably pointless to try to do anything here, unlikely to be
+ // performance-affecting.
+ if (F.doesNotReturn()) {
+ for (auto &BB : F)
+ for (auto &I : make_early_inc_range(BB))
+ if (isa<InstrProfCntrInstBase>(&I))
+ I.eraseFromParent();
+ return true;
+ }
+
auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(F);
diff --git a/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll b/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
index 75f292deb71c2..8f72711a9c8b1 100644
--- a/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
+++ b/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
@@ -310,11 +310,26 @@ define void @has_musttail_calls() {
musttail call void @bar()
ret void
}
+
+define void @does_not_return() noreturn {
+; INSTRUMENT-LABEL: define void @does_not_return(
+; INSTRUMENT-SAME: ) #[[ATTR0:[0-9]+]] {
+; INSTRUMENT-NEXT: call void @llvm.instrprof.increment(ptr @does_not_return, i64 742261418966908927, i32 1, i32 0)
+; INSTRUMENT-NEXT: unreachable
+;
+; LOWERING-LABEL: define void @does_not_return(
+; LOWERING-SAME: ) #[[ATTR0:[0-9]+]] !guid [[META8:![0-9]+]] {
+; LOWERING-NEXT: unreachable
+;
+ unreachable
+}
;.
-; LOWERING: attributes #[[ATTR0:[0-9]+]] = { nounwind }
-; LOWERING: attributes #[[ATTR1:[0-9]+]] = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+; LOWERING: attributes #[[ATTR0]] = { noreturn }
+; LOWERING: attributes #[[ATTR1:[0-9]+]] = { nounwind }
+; LOWERING: attributes #[[ATTR2:[0-9]+]] = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
;.
-; INSTRUMENT: attributes #[[ATTR0:[0-9]+]] = { nounwind }
+; INSTRUMENT: attributes #[[ATTR0]] = { noreturn }
+; INSTRUMENT: attributes #[[ATTR1:[0-9]+]] = { nounwind }
;.
; LOWERING: [[META0]] = !{i64 6699318081062747564}
; LOWERING: [[META1]] = !{i64 4909520559318251808}
@@ -324,4 +339,5 @@ define void @has_musttail_calls() {
; LOWERING: [[META5]] = !{i64 5458232184388660970}
; LOWERING: [[META6]] = !{i64 -3771893999295659109}
; LOWERING: [[META7]] = !{i64 -4680624981836544329}
+; LOWERING: [[META8]] = !{i64 5519225910966780583}
;.
|
@llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesFull diff: https://github.com/llvm/llvm-project/pull/134932.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp b/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
index 2f8d7766bb588..a314457423819 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
@@ -8,6 +8,7 @@
//
#include "llvm/Transforms/Instrumentation/PGOCtxProfLowering.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Analysis/CtxProfAnalysis.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/IR/Analysis.h"
@@ -206,6 +207,17 @@ PreservedAnalyses PGOCtxProfLoweringPass::run(Module &M,
bool CtxInstrumentationLowerer::lowerFunction(Function &F) {
if (F.isDeclaration())
return false;
+
+ // Probably pointless to try to do anything here, unlikely to be
+ // performance-affecting.
+ if (F.doesNotReturn()) {
+ for (auto &BB : F)
+ for (auto &I : make_early_inc_range(BB))
+ if (isa<InstrProfCntrInstBase>(&I))
+ I.eraseFromParent();
+ return true;
+ }
+
auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(F);
diff --git a/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll b/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
index 75f292deb71c2..8f72711a9c8b1 100644
--- a/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
+++ b/llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll
@@ -310,11 +310,26 @@ define void @has_musttail_calls() {
musttail call void @bar()
ret void
}
+
+define void @does_not_return() noreturn {
+; INSTRUMENT-LABEL: define void @does_not_return(
+; INSTRUMENT-SAME: ) #[[ATTR0:[0-9]+]] {
+; INSTRUMENT-NEXT: call void @llvm.instrprof.increment(ptr @does_not_return, i64 742261418966908927, i32 1, i32 0)
+; INSTRUMENT-NEXT: unreachable
+;
+; LOWERING-LABEL: define void @does_not_return(
+; LOWERING-SAME: ) #[[ATTR0:[0-9]+]] !guid [[META8:![0-9]+]] {
+; LOWERING-NEXT: unreachable
+;
+ unreachable
+}
;.
-; LOWERING: attributes #[[ATTR0:[0-9]+]] = { nounwind }
-; LOWERING: attributes #[[ATTR1:[0-9]+]] = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+; LOWERING: attributes #[[ATTR0]] = { noreturn }
+; LOWERING: attributes #[[ATTR1:[0-9]+]] = { nounwind }
+; LOWERING: attributes #[[ATTR2:[0-9]+]] = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
;.
-; INSTRUMENT: attributes #[[ATTR0:[0-9]+]] = { nounwind }
+; INSTRUMENT: attributes #[[ATTR0]] = { noreturn }
+; INSTRUMENT: attributes #[[ATTR1:[0-9]+]] = { nounwind }
;.
; LOWERING: [[META0]] = !{i64 6699318081062747564}
; LOWERING: [[META1]] = !{i64 4909520559318251808}
@@ -324,4 +339,5 @@ define void @has_musttail_calls() {
; LOWERING: [[META5]] = !{i64 5458232184388660970}
; LOWERING: [[META6]] = !{i64 -3771893999295659109}
; LOWERING: [[META7]] = !{i64 -4680624981836544329}
+; LOWERING: [[META8]] = !{i64 5519225910966780583}
;.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The tests pass locally w/ this change.
Merge activity
|
…134932) `noreturn` functions are doubtfully interesting for performance optimization / profiling.
noreturn
functions are doubtfully interesting for performance optimization / profiling.