Skip to content

release/19.x: Revert "demangle function names in trace files (#87626)" #102552

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

Merged
merged 1 commit into from
Aug 10, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Aug 8, 2024

Backport 72b73e2

Requested by: @MaskRay

@llvmbot llvmbot requested review from rupprecht and keith as code owners August 8, 2024 23:41
@llvmbot llvmbot added this to the LLVM 19.X Release milestone Aug 8, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Aug 8, 2024

@nikic What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from nikic August 8, 2024 23:41
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' bazel "Peripheral" support tier build system: utils/bazel llvm:ir labels Aug 8, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Aug 8, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-llvm-ir

Author: None (llvmbot)

Changes

Backport 72b73e2

Requested by: @MaskRay


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

5 Files Affected:

  • (modified) clang/test/Driver/ftime-trace-sections.py (-11)
  • (modified) llvm/lib/IR/LegacyPassManager.cpp (+1-3)
  • (modified) llvm/lib/Passes/CMakeLists.txt (-1)
  • (modified) llvm/lib/Passes/StandardInstrumentations.cpp (+4-5)
  • (modified) utils/bazel/llvm-project-overlay/llvm/BUILD.bazel (-1)
diff --git a/clang/test/Driver/ftime-trace-sections.py b/clang/test/Driver/ftime-trace-sections.py
old mode 100755
new mode 100644
index b332931d29a622..02afa4ac54eb7b
--- a/clang/test/Driver/ftime-trace-sections.py
+++ b/clang/test/Driver/ftime-trace-sections.py
@@ -19,10 +19,7 @@ def is_before(range1, range2):
 
 log_contents = json.loads(sys.stdin.read())
 events = log_contents["traceEvents"]
-
-instants = [event for event in events if event["name"] == "InstantiateFunction"]
 codegens = [event for event in events if event["name"] == "CodeGen Function"]
-opts = [event for event in events if event["name"] == "OptFunction"]
 frontends = [event for event in events if event["name"] == "Frontend"]
 backends = [event for event in events if event["name"] == "Backend"]
 
@@ -51,11 +48,3 @@ def is_before(range1, range2):
     ]
 ):
     sys.exit("Not all Frontend section are before all Backend sections!")
-
-# Check that entries for foo exist and are in a demangled form.
-if not any(e for e in instants if "foo<int>" in e["args"]["detail"]):
-    sys.exit("Missing Instantiate entry for foo!")
-if not any(e for e in codegens if "foo<int>" in e["args"]["detail"]):
-    sys.exit("Missing CodeGen entry for foo!")
-if not any(e for e in opts if "foo<int>" in e["args"]["detail"]):
-    sys.exit("Missing Optimize entry for foo!")
diff --git a/llvm/lib/IR/LegacyPassManager.cpp b/llvm/lib/IR/LegacyPassManager.cpp
index 9c44eff7953ac8..01aaedcf7d5473 100644
--- a/llvm/lib/IR/LegacyPassManager.cpp
+++ b/llvm/lib/IR/LegacyPassManager.cpp
@@ -12,7 +12,6 @@
 
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/ADT/MapVector.h"
-#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LLVMContext.h"
@@ -1416,8 +1415,7 @@ bool FPPassManager::runOnFunction(Function &F) {
 
   // Store name outside of loop to avoid redundant calls.
   const StringRef Name = F.getName();
-  llvm::TimeTraceScope FunctionScope(
-      "OptFunction", [&F]() { return demangle(F.getName().str()); });
+  llvm::TimeTraceScope FunctionScope("OptFunction", Name);
 
   for (unsigned Index = 0; Index < getNumContainedPasses(); ++Index) {
     FunctionPass *FP = getContainedPass(Index);
diff --git a/llvm/lib/Passes/CMakeLists.txt b/llvm/lib/Passes/CMakeLists.txt
index b5224327d79216..6425f4934b2103 100644
--- a/llvm/lib/Passes/CMakeLists.txt
+++ b/llvm/lib/Passes/CMakeLists.txt
@@ -21,7 +21,6 @@ add_llvm_component_library(LLVMPasses
   CodeGen
   Core
   Coroutines
-  Demangle
   HipStdPar
   IPO
   InstCombine
diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
index fc7b82d522bf0e..4eff2deef9abc5 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -23,7 +23,6 @@
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/MachineVerifier.h"
-#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Module.h"
@@ -236,12 +235,12 @@ void printIR(raw_ostream &OS, const MachineFunction *MF) {
   MF->print(OS);
 }
 
-std::string getIRName(Any IR, bool demangled = false) {
+std::string getIRName(Any IR) {
   if (unwrapIR<Module>(IR))
     return "[module]";
 
   if (const auto *F = unwrapIR<Function>(IR))
-    return demangled ? demangle(F->getName()) : F->getName().str();
+    return F->getName().str();
 
   if (const auto *C = unwrapIR<LazyCallGraph::SCC>(IR))
     return C->getName();
@@ -251,7 +250,7 @@ std::string getIRName(Any IR, bool demangled = false) {
            L->getHeader()->getParent()->getName().str();
 
   if (const auto *MF = unwrapIR<MachineFunction>(IR))
-    return demangled ? demangle(MF->getName()) : MF->getName().str();
+    return MF->getName().str();
 
   llvm_unreachable("Unknown wrapped IR type");
 }
@@ -1589,7 +1588,7 @@ void TimeProfilingPassesHandler::registerCallbacks(
 }
 
 void TimeProfilingPassesHandler::runBeforePass(StringRef PassID, Any IR) {
-  timeTraceProfilerBegin(PassID, getIRName(IR, true));
+  timeTraceProfilerBegin(PassID, getIRName(IR));
 }
 
 void TimeProfilingPassesHandler::runAfterPass() { timeTraceProfilerEnd(); }
diff --git a/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel b/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
index 4d443e809d55bd..4a59c16ba12fcc 100644
--- a/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
@@ -2653,7 +2653,6 @@ cc_library(
         ":CodeGen",
         ":Core",
         ":Coroutines",
-        ":Demangle",
         ":HipStdPar",
         ":IPO",
         ":IRPrinter",

This reverts commit 0fa20c5.

Storing raw symbol names is generally preferred in profile files.
Demangling might lose information. Language frontends might use
demangling schemes not supported by LLVMDemangle
(llvm#45901 (comment)).
In addition, calling `demangle` for each function has a significant
performance overhead (llvm#102222).

I believe that even if we decide to provide a producer-side demangling,
it would not be on by default.

Pull Request: llvm#102274

(cherry picked from commit 72b73e2)
@tru tru merged commit d893708 into llvm:release/19.x Aug 10, 2024
12 of 14 checks passed
Copy link

@MaskRay (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel "Peripheral" support tier build system: utils/bazel clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category llvm:ir
Projects
Development

Successfully merging this pull request may close these issues.

4 participants