[llvm] [MemProf] Fix tailcall discovery checking for multiple callee chains (PR #92632)
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Fri May 24 07:36:35 PDT 2024
https://github.com/teresajohnson updated https://github.com/llvm/llvm-project/pull/92632
>From 7546388c69075ae709be582d87364c3bb3eaf018 Mon Sep 17 00:00:00 2001
From: Teresa Johnson <tejohnson at google.com>
Date: Fri, 17 May 2024 21:33:23 -0700
Subject: [PATCH 1/2] [MemProf] Fix tailcall discovery checking for multiple
callee chains
When looking for missing frames due to tail calls, we were not checking
the output parameter of the recursive call in the correct place.
Make sure we check for the case when that recursive call returned false
due to multiple possible callee chains.
Extended the existing test a bit to catch this case.
---
.../IPO/MemProfContextDisambiguation.cpp | 12 +++---
.../ThinLTO/X86/memprof-tailcall-nonunique.ll | 41 +++++++++----------
.../tailcall-nonunique.ll | 35 ++++++++--------
3 files changed, 42 insertions(+), 46 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
index b9d84d583f495..23e067714e754 100644
--- a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
+++ b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
@@ -1889,15 +1889,15 @@ bool ModuleCallsiteContextGraph::findProfiledCalleeThroughTailCalls(
} else if (findProfiledCalleeThroughTailCalls(
ProfiledCallee, CalledFunction, Depth + 1,
FoundCalleeChain, FoundMultipleCalleeChains)) {
- if (FoundMultipleCalleeChains)
- return false;
+ assert(!FoundMultipleCalleeChains);
if (FoundSingleCalleeChain) {
FoundMultipleCalleeChains = true;
return false;
}
FoundSingleCalleeChain = true;
SaveCallsiteInfo(&I, CalleeFunc);
- }
+ } else if (FoundMultipleCalleeChains)
+ return false;
}
}
@@ -2004,8 +2004,7 @@ bool IndexCallsiteContextGraph::findProfiledCalleeThroughTailCalls(
} else if (findProfiledCalleeThroughTailCalls(
ProfiledCallee, CallEdge.first, Depth + 1,
FoundCalleeChain, FoundMultipleCalleeChains)) {
- if (FoundMultipleCalleeChains)
- return false;
+ assert(!FoundMultipleCalleeChains);
if (FoundSingleCalleeChain) {
FoundMultipleCalleeChains = true;
return false;
@@ -2015,7 +2014,8 @@ bool IndexCallsiteContextGraph::findProfiledCalleeThroughTailCalls(
// Add FS to FSToVIMap in case it isn't already there.
assert(!FSToVIMap.count(FS) || FSToVIMap[FS] == FSVI);
FSToVIMap[FS] = FSVI;
- }
+ } else if (FoundMultipleCalleeChains)
+ return false;
}
}
diff --git a/llvm/test/ThinLTO/X86/memprof-tailcall-nonunique.ll b/llvm/test/ThinLTO/X86/memprof-tailcall-nonunique.ll
index d7cfafec89feb..49c22bf590e6b 100644
--- a/llvm/test/ThinLTO/X86/memprof-tailcall-nonunique.ll
+++ b/llvm/test/ThinLTO/X86/memprof-tailcall-nonunique.ll
@@ -14,10 +14,11 @@
; RUN: -r=%t.o,_Z4baz1v,plx \
; RUN: -r=%t.o,_Z4baz2v,plx \
; RUN: -r=%t.o,_Z3foob,plx \
+; RUN: -r=%t.o,xyz,plx \
; RUN: -r=%t.o,main,plx \
; RUN: -r=%t.o,_Znam, \
; RUN: -stats -debug -save-temps \
-; RUN: -o %t.out 2>&1 | FileCheck %s --check-prefix=STATS
+; RUN: -o %t.out 2>&1 | FileCheck %s --check-prefix=STATS --check-prefix=DEBUG
; RUN: llvm-dis %t.out.1.4.opt.bc -o - | FileCheck %s --check-prefix=IR
@@ -31,22 +32,20 @@
; RUN: -r=%t.o,_Z4baz1v,plx \
; RUN: -r=%t.o,_Z4baz2v,plx \
; RUN: -r=%t.o,_Z3foob,plx \
+; RUN: -r=%t.o,xyz,plx \
; RUN: -r=%t.o,main,plx \
; RUN: -r=%t.o,_Znam, \
; RUN: -stats -debug \
-; RUN: -o %t2.out 2>&1 | FileCheck %s --check-prefix=STATS
+; RUN: -o %t2.out 2>&1 | FileCheck %s --check-prefix=STATS --check-prefix=DEBUG
;; Run ThinLTO backend
; RUN: opt -passes=memprof-context-disambiguation \
; RUN: -memprof-import-summary=%t.o.thinlto.bc \
; RUN: -stats %t.o -S 2>&1 | FileCheck %s --check-prefix=IR
-; DEBUG: Not found through unique tail call chain: _Z3barv from main that actually called _Z3foob (found multiple possible chains)
-; DEBUG: Not found through unique tail call chain: _Z3barv from main that actually called _Z3foob (found multiple possible chains)
-; DEBUG: Not found through unique tail call chain: _Z3barv from main that actually called _Z3foob (found multiple possible chains)
-; DEBUG: Not found through unique tail call chain: _Z3barv from main that actually called _Z3foob (found multiple possible chains)
+; DEBUG: Not found through unique tail call chain: 17377440600225628772 (_Z3barv) from 15822663052811949562 (main) that actually called 8716735811002003409 (xyz) (found multiple possible chains)
-; STATS: 4 memprof-context-disambiguation - Number of profiled callees found via multiple tail call chains
+; STATS: 1 memprof-context-disambiguation - Number of profiled callees found via multiple tail call chains
;; Check that all calls in the IR are to the original functions, leading to a
;; non-cold operator new call.
@@ -125,17 +124,24 @@ return: ; preds = %if.else, %if.then
}
; Function Attrs: noinline
-; IR-LABEL: @main()
-define dso_local i32 @main() local_unnamed_addr #0 {
+; IR-LABEL: @xyz()
+define dso_local i32 @xyz() local_unnamed_addr #0 {
delete.end13:
; IR: call ptr @_Z3foob(i1 true)
- %call = tail call ptr @_Z3foob(i1 true), !callsite !10
+ %call = tail call ptr @_Z3foob(i1 true)
; IR: call ptr @_Z3foob(i1 true)
- %call1 = tail call ptr @_Z3foob(i1 true), !callsite !11
+ %call1 = tail call ptr @_Z3foob(i1 true)
; IR: call ptr @_Z3foob(i1 false)
- %call2 = tail call ptr @_Z3foob(i1 false), !callsite !12
+ %call2 = tail call ptr @_Z3foob(i1 false)
; IR: call ptr @_Z3foob(i1 false)
- %call3 = tail call ptr @_Z3foob(i1 false), !callsite !13
+ %call3 = tail call ptr @_Z3foob(i1 false)
+ ret i32 0
+}
+
+define dso_local i32 @main() local_unnamed_addr #0 {
+delete.end13:
+ ; IR: call i32 @xyz()
+ %call1 = tail call i32 @xyz(), !callsite !11
ret i32 0
}
@@ -145,17 +151,10 @@ attributes #0 = { noinline }
attributes #1 = { nobuiltin allocsize(0) }
attributes #2 = { builtin allocsize(0) }
-!0 = !{!1, !3, !5, !7}
-!1 = !{!2, !"notcold"}
-!2 = !{i64 3186456655321080972, i64 6307901912192269588}
-!3 = !{!4, !"cold"}
-!4 = !{i64 3186456655321080972, i64 6792096022461663180}
+!0 = !{!5, !7}
!5 = !{!6, !"notcold"}
!6 = !{i64 3186456655321080972, i64 8632435727821051414}
!7 = !{!8, !"cold"}
!8 = !{i64 3186456655321080972, i64 -3421689549917153178}
!9 = !{i64 3186456655321080972}
-!10 = !{i64 8632435727821051414}
!11 = !{i64 -3421689549917153178}
-!12 = !{i64 6307901912192269588}
-!13 = !{i64 6792096022461663180}
diff --git a/llvm/test/Transforms/MemProfContextDisambiguation/tailcall-nonunique.ll b/llvm/test/Transforms/MemProfContextDisambiguation/tailcall-nonunique.ll
index b49c9a139cfc1..13c056fc0b8c3 100644
--- a/llvm/test/Transforms/MemProfContextDisambiguation/tailcall-nonunique.ll
+++ b/llvm/test/Transforms/MemProfContextDisambiguation/tailcall-nonunique.ll
@@ -9,10 +9,7 @@
; RUN: -stats -debug %s -S 2>&1 | FileCheck %s --check-prefix=STATS \
; RUN: --check-prefix=IR --check-prefix=DEBUG
-; DEBUG: Not found through unique tail call chain: _Z3barv from main that actually called _Z3foob (found multiple possible chains)
-; DEBUG: Not found through unique tail call chain: _Z3barv from main that actually called _Z3foob (found multiple possible chains)
-; DEBUG: Not found through unique tail call chain: _Z3barv from main that actually called _Z3foob (found multiple possible chains)
-; DEBUG: Not found through unique tail call chain: _Z3barv from main that actually called _Z3foob (found multiple possible chains)
+; DEBUG: Not found through unique tail call chain: _Z3barv from main that actually called xyz (found multiple possible chains)
;; Check that all calls in the IR are to the original functions, leading to a
;; non-cold operator new call.
@@ -91,39 +88,39 @@ return: ; preds = %if.else, %if.then
}
; Function Attrs: noinline
-; IR-LABEL: @main()
-define dso_local i32 @main() local_unnamed_addr #0 {
+; IR-LABEL: @xyz()
+define dso_local i32 @xyz() local_unnamed_addr #0 {
delete.end13:
; IR: call ptr @_Z3foob(i1 true)
- %call = tail call ptr @_Z3foob(i1 true), !callsite !10
+ %call = tail call ptr @_Z3foob(i1 true)
; IR: call ptr @_Z3foob(i1 true)
- %call1 = tail call ptr @_Z3foob(i1 true), !callsite !11
+ %call1 = tail call ptr @_Z3foob(i1 true)
; IR: call ptr @_Z3foob(i1 false)
- %call2 = tail call ptr @_Z3foob(i1 false), !callsite !12
+ %call2 = tail call ptr @_Z3foob(i1 false)
; IR: call ptr @_Z3foob(i1 false)
- %call3 = tail call ptr @_Z3foob(i1 false), !callsite !13
+ %call3 = tail call ptr @_Z3foob(i1 false)
+ ret i32 0
+}
+
+define dso_local i32 @main() local_unnamed_addr #0 {
+delete.end13:
+ ; IR: call i32 @xyz()
+ %call1 = tail call i32 @xyz(), !callsite !11
ret i32 0
}
; IR: attributes #[[NOTCOLD]] = { builtin allocsize(0) "memprof"="notcold" }
-; STATS: 4 memprof-context-disambiguation - Number of profiled callees found via multiple tail call chains
+; STATS: 1 memprof-context-disambiguation - Number of profiled callees found via multiple tail call chains
attributes #0 = { noinline }
attributes #1 = { nobuiltin allocsize(0) }
attributes #2 = { builtin allocsize(0) }
-!0 = !{!1, !3, !5, !7}
-!1 = !{!2, !"notcold"}
-!2 = !{i64 3186456655321080972, i64 6307901912192269588}
-!3 = !{!4, !"cold"}
-!4 = !{i64 3186456655321080972, i64 6792096022461663180}
+!0 = !{!5, !7}
!5 = !{!6, !"notcold"}
!6 = !{i64 3186456655321080972, i64 8632435727821051414}
!7 = !{!8, !"cold"}
!8 = !{i64 3186456655321080972, i64 -3421689549917153178}
!9 = !{i64 3186456655321080972}
-!10 = !{i64 8632435727821051414}
!11 = !{i64 -3421689549917153178}
-!12 = !{i64 6307901912192269588}
-!13 = !{i64 6792096022461663180}
>From b1df65f6ae3dda013ab30230c245fe743054cfe5 Mon Sep 17 00:00:00 2001
From: Teresa Johnson <tejohnson at google.com>
Date: Fri, 24 May 2024 07:35:53 -0700
Subject: [PATCH 2/2] Remove unreferenced labels
---
llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp | 4 ++++
.../MemProfContextDisambiguation/tailcall-nonunique.ll | 2 --
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
index 23e067714e754..c53b9451625c6 100644
--- a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
+++ b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
@@ -1889,6 +1889,8 @@ bool ModuleCallsiteContextGraph::findProfiledCalleeThroughTailCalls(
} else if (findProfiledCalleeThroughTailCalls(
ProfiledCallee, CalledFunction, Depth + 1,
FoundCalleeChain, FoundMultipleCalleeChains)) {
+ // findProfiledCalleeThroughTailCalls should not have returned
+ // true if FoundMultipleCalleeChains.
assert(!FoundMultipleCalleeChains);
if (FoundSingleCalleeChain) {
FoundMultipleCalleeChains = true;
@@ -2004,6 +2006,8 @@ bool IndexCallsiteContextGraph::findProfiledCalleeThroughTailCalls(
} else if (findProfiledCalleeThroughTailCalls(
ProfiledCallee, CallEdge.first, Depth + 1,
FoundCalleeChain, FoundMultipleCalleeChains)) {
+ // findProfiledCalleeThroughTailCalls should not have returned
+ // true if FoundMultipleCalleeChains.
assert(!FoundMultipleCalleeChains);
if (FoundSingleCalleeChain) {
FoundMultipleCalleeChains = true;
diff --git a/llvm/test/Transforms/MemProfContextDisambiguation/tailcall-nonunique.ll b/llvm/test/Transforms/MemProfContextDisambiguation/tailcall-nonunique.ll
index 13c056fc0b8c3..985c381ad42ff 100644
--- a/llvm/test/Transforms/MemProfContextDisambiguation/tailcall-nonunique.ll
+++ b/llvm/test/Transforms/MemProfContextDisambiguation/tailcall-nonunique.ll
@@ -90,7 +90,6 @@ return: ; preds = %if.else, %if.then
; Function Attrs: noinline
; IR-LABEL: @xyz()
define dso_local i32 @xyz() local_unnamed_addr #0 {
-delete.end13:
; IR: call ptr @_Z3foob(i1 true)
%call = tail call ptr @_Z3foob(i1 true)
; IR: call ptr @_Z3foob(i1 true)
@@ -103,7 +102,6 @@ delete.end13:
}
define dso_local i32 @main() local_unnamed_addr #0 {
-delete.end13:
; IR: call i32 @xyz()
%call1 = tail call i32 @xyz(), !callsite !11
ret i32 0
More information about the llvm-commits
mailing list