From 4c8f79267779327a13d9953e1aaf240cd1af6efe Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Thu, 9 Jan 2025 11:55:12 -0800 Subject: [PATCH] [MemProf] Disable cloning of callsites in recursive cycles by default This disables the support added in PR121985 by default while we investigate a compile time crash. --- .../IPO/MemProfContextDisambiguation.cpp | 4 ++-- llvm/test/ThinLTO/X86/memprof-recursive.ll | 19 ++++++++++++++++++- .../MemProfContextDisambiguation/recursive.ll | 14 +++++++++++++- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp index 016db55c99c3e..3d0e5cb5f97b4 100644 --- a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp +++ b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp @@ -122,9 +122,9 @@ static cl::opt cl::desc("Max depth to recursively search for missing " "frames through tail calls.")); -// By default enable cloning of callsites involved with recursive cycles +// Optionally enable cloning of callsites involved with recursive cycles static cl::opt AllowRecursiveCallsites( - "memprof-allow-recursive-callsites", cl::init(true), cl::Hidden, + "memprof-allow-recursive-callsites", cl::init(false), cl::Hidden, cl::desc("Allow cloning of callsites involved in recursive cycles")); // When disabled, try to detect and prevent cloning of recursive contexts. diff --git a/llvm/test/ThinLTO/X86/memprof-recursive.ll b/llvm/test/ThinLTO/X86/memprof-recursive.ll index 2b1d7081b7610..1f4f75460b070 100644 --- a/llvm/test/ThinLTO/X86/memprof-recursive.ll +++ b/llvm/test/ThinLTO/X86/memprof-recursive.ll @@ -5,7 +5,7 @@ ; RUN: opt -thinlto-bc %s >%t.o -;; By default we should enable cloning of contexts involved with recursive +;; Check behavior when we enable cloning of contexts involved with recursive ;; cycles, but not through the cycle itself. I.e. until full support for ;; recursion is added, the cloned recursive call from C back to B (line 12) will ;; not be updated to call a clone. @@ -18,6 +18,7 @@ ; RUN: -r=%t.o,_Znam, \ ; RUN: -memprof-verify-ccg -memprof-verify-nodes \ ; RUN: -pass-remarks=memprof-context-disambiguation \ +; RUN: -memprof-allow-recursive-callsites=true \ ; RUN: -o %t.out 2>&1 | FileCheck %s \ ; RUN: --implicit-check-not "memprof_recursive3.cc:12:10: call in clone _Z1Ci.memprof.1 assigned" \ ; RUN: --check-prefix=ALLOW-RECUR-CALLSITES --check-prefix=ALLOW-RECUR-CONTEXTS @@ -38,6 +39,21 @@ ; RUN: --implicit-check-not="created clone" \ ; RUN: --implicit-check-not="marked with memprof allocation attribute cold" +;; Check the default behavior (disabled recursive callsites). +; RUN: llvm-lto2 run %t.o -enable-memprof-context-disambiguation \ +; RUN: -supports-hot-cold-new \ +; RUN: -r=%t.o,_Z1Dv,plx \ +; RUN: -r=%t.o,_Z1Ci,plx \ +; RUN: -r=%t.o,_Z1Bi,plx \ +; RUN: -r=%t.o,main,plx \ +; RUN: -r=%t.o,_Znam, \ +; RUN: -memprof-verify-ccg -memprof-verify-nodes \ +; RUN: -pass-remarks=memprof-context-disambiguation \ +; RUN: -o %t.out 2>&1 | FileCheck %s --allow-empty \ +; RUN: --implicit-check-not "memprof_recursive3.cc:12:10: call in clone _Z1Ci.memprof.1 assigned" \ +; RUN: --implicit-check-not="created clone" \ +; RUN: --implicit-check-not="marked with memprof allocation attribute cold" + ;; Skipping recursive contexts should prevent spurious call to cloned version of ;; B from the context starting at memprof_recursive.cc:19:13, which is actually ;; recursive (until that support is added). @@ -50,6 +66,7 @@ ; RUN: -r=%t.o,_Znam, \ ; RUN: -memprof-verify-ccg -memprof-verify-nodes \ ; RUN: -pass-remarks=memprof-context-disambiguation \ +; RUN: -memprof-allow-recursive-callsites=true \ ; RUN: -memprof-allow-recursive-contexts=false \ ; RUN: -o %t.out 2>&1 | FileCheck %s \ ; RUN: --implicit-check-not "memprof_recursive3.cc:12:10: call in clone _Z1Ci.memprof.1 assigned" \ diff --git a/llvm/test/Transforms/MemProfContextDisambiguation/recursive.ll b/llvm/test/Transforms/MemProfContextDisambiguation/recursive.ll index 759d5115896c1..fd87cb535c42e 100644 --- a/llvm/test/Transforms/MemProfContextDisambiguation/recursive.ll +++ b/llvm/test/Transforms/MemProfContextDisambiguation/recursive.ll @@ -34,13 +34,14 @@ ;; ;; The IR was then reduced using llvm-reduce with the expected FileCheck input. -;; By default we should enable cloning of contexts involved with recursive +;; Check behavior when we enable cloning of contexts involved with recursive ;; cycles, but not through the cycle itself. I.e. until full support for ;; recursion is added, the cloned recursive call from C back to B (line 12) will ;; not be updated to call a clone. ; RUN: opt -passes=memprof-context-disambiguation -supports-hot-cold-new \ ; RUN: -memprof-verify-ccg -memprof-verify-nodes \ ; RUN: -pass-remarks=memprof-context-disambiguation \ +; RUN: -memprof-allow-recursive-callsites=true \ ; RUN: %s -S 2>&1 | FileCheck %s \ ; RUN: --implicit-check-not "memprof_recursive3.cc:12:10: call in clone _Z1Ci.memprof.1 assigned" \ ; RUN: --check-prefix=ALL --check-prefix=ALLOW-RECUR-CALLSITES --check-prefix=ALLOW-RECUR-CONTEXTS @@ -56,12 +57,23 @@ ; RUN: --implicit-check-not="marked with memprof allocation attribute cold" \ ; RUN: --check-prefix=ALL +;; Check the default behavior (disabled recursive callsites). +; RUN: opt -passes=memprof-context-disambiguation -supports-hot-cold-new \ +; RUN: -memprof-verify-ccg -memprof-verify-nodes \ +; RUN: -pass-remarks=memprof-context-disambiguation \ +; RUN: %s -S 2>&1 | FileCheck %s \ +; RUN: --implicit-check-not "memprof_recursive3.cc:12:10: call in clone _Z1Ci.memprof.1 assigned" \ +; RUN: --implicit-check-not="created clone" \ +; RUN: --implicit-check-not="marked with memprof allocation attribute cold" \ +; RUN: --check-prefix=ALL + ;; Skipping recursive contexts should prevent spurious call to cloned version of ;; B from the context starting at memprof_recursive.cc:19:13, which is actually ;; recursive (until that support is added). ; RUN: opt -passes=memprof-context-disambiguation -supports-hot-cold-new \ ; RUN: -memprof-verify-ccg -memprof-verify-nodes \ ; RUN: -pass-remarks=memprof-context-disambiguation \ +; RUN: -memprof-allow-recursive-callsites=true \ ; RUN: -memprof-allow-recursive-contexts=false \ ; RUN: %s -S 2>&1 | FileCheck %s \ ; RUN: --implicit-check-not "memprof_recursive3.cc:12:10: call in clone _Z1Ci.memprof.1 assigned" \