Skip to content

[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

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented Apr 8, 2025

noreturn functions are doubtfully interesting for performance optimization / profiling.

Copy link
Member Author

mtrofin commented Apr 8, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@mtrofin mtrofin marked this pull request as ready for review April 8, 2025 21:21
@llvmbot llvmbot added PGO Profile Guided Optimizations llvm:transforms labels Apr 8, 2025
@mtrofin mtrofin requested a review from ilovepi April 8, 2025 21:21
@llvmbot
Copy link
Member

llvmbot commented Apr 8, 2025

@llvm/pr-subscribers-pgo

Author: Mircea Trofin (mtrofin)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/134932.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp (+12)
  • (modified) llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll (+19-3)
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}
 ;.

@llvmbot
Copy link
Member

llvmbot commented Apr 8, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/134932.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp (+12)
  • (modified) llvm/test/Transforms/PGOProfile/ctx-instrumentation.ll (+19-3)
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}
 ;.

Copy link
Contributor

@ilovepi ilovepi left a 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.

Copy link
Member Author

mtrofin commented Apr 8, 2025

Merge activity

  • Apr 8, 5:44 PM EDT: A user started a stack merge that includes this pull request via Graphite.

@mtrofin mtrofin merged commit cfa6a59 into main Apr 8, 2025
12 of 15 checks passed
@mtrofin mtrofin deleted the users/mtrofin/04-08-_ctxprof_don_t_lower_instrumentation_for_noreturn_functions branch April 8, 2025 21:48
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
…134932)

`noreturn` functions are doubtfully interesting for performance optimization / profiling.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:transforms PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants