Skip to content
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

[RemoveDIs] Load into new debug info format by default in llvm-lto and llvm-lto2 #86271

Merged
merged 2 commits into from
Mar 22, 2024

Conversation

OCHyams
Copy link
Contributor

@OCHyams OCHyams commented Mar 22, 2024

Directly load all bitcode into the new debug info format from llvm-lto and llvm-lto2. This means that new-mode bitcode no longer round-trips back to old-mode after parsing, and that old-mode bitcode gets auto-upgraded to new-mode debug info (which is the current in-memory default in LLVM).

@llvmbot llvmbot added the LTO Link time optimization (regular/full LTO or ThinLTO) label Mar 22, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 22, 2024

@llvm/pr-subscribers-lto

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

Directly load all bitcode into the new debug info format from llvm-lto and llvm-lto2. This means that new-mode bitcode no longer round-trips back to old-mode after parsing, and that old-mode bitcode gets auto-upgraded to new-mode debug info (which is the current in-memory default in LLVM).


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

3 Files Affected:

  • (modified) llvm/lib/LTO/LTO.cpp (+3-1)
  • (modified) llvm/tools/llvm-lto/llvm-lto.cpp (+4)
  • (modified) llvm/tools/llvm-lto2/llvm-lto2.cpp (+4)
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index b58418c64a116b..53060df7f503e0 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -579,7 +579,9 @@ LTO::RegularLTOState::RegularLTOState(unsigned ParallelCodeGenParallelismLevel,
                                       const Config &Conf)
     : ParallelCodeGenParallelismLevel(ParallelCodeGenParallelismLevel),
       Ctx(Conf), CombinedModule(std::make_unique<Module>("ld-temp.o", Ctx)),
-      Mover(std::make_unique<IRMover>(*CombinedModule)) {}
+      Mover(std::make_unique<IRMover>(*CombinedModule)) {
+  CombinedModule->IsNewDbgInfoFormat = UseNewDbgInfoFormat;
+}
 
 LTO::ThinLTOState::ThinLTOState(ThinBackend Backend)
     : Backend(Backend), CombinedIndex(/*HaveGVs*/ false) {
diff --git a/llvm/tools/llvm-lto/llvm-lto.cpp b/llvm/tools/llvm-lto/llvm-lto.cpp
index 7943d6952b828d..1405bd23c99106 100644
--- a/llvm/tools/llvm-lto/llvm-lto.cpp
+++ b/llvm/tools/llvm-lto/llvm-lto.cpp
@@ -270,6 +270,7 @@ static cl::opt<bool> TryUseNewDbgInfoFormat(
     cl::init(false), cl::Hidden);
 
 extern cl::opt<bool> UseNewDbgInfoFormat;
+extern cl::opt<cl::boolOrDefault> LoadBitcodeIntoNewDbgInforFormat;
 
 namespace {
 
@@ -943,6 +944,9 @@ int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
   cl::HideUnrelatedOptions({&LTOCategory, &getColorCategory()});
   cl::ParseCommandLineOptions(argc, argv, "llvm LTO linker\n");
+  // Load bitcode into the new debug info format by default.
+  if (LoadBitcodeIntoNewDbgInforFormat == cl::boolOrDefault::BOU_UNSET)
+    LoadBitcodeIntoNewDbgInforFormat = cl::boolOrDefault::BOU_TRUE;
 
   // RemoveDIs debug-info transition: tests may request that we /try/ to use the
   // new debug-info format.
diff --git a/llvm/tools/llvm-lto2/llvm-lto2.cpp b/llvm/tools/llvm-lto2/llvm-lto2.cpp
index d5de4f6b1a277c..b08d1fdd704828 100644
--- a/llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ b/llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -193,6 +193,7 @@ static cl::opt<bool> TryUseNewDbgInfoFormat(
     cl::init(false), cl::Hidden);
 
 extern cl::opt<bool> UseNewDbgInfoFormat;
+extern cl::opt<cl::boolOrDefault> LoadBitcodeIntoNewDbgInforFormat;
 
 static void check(Error E, std::string Msg) {
   if (!E)
@@ -228,6 +229,9 @@ static int usage() {
 
 static int run(int argc, char **argv) {
   cl::ParseCommandLineOptions(argc, argv, "Resolution-based LTO test harness");
+  // Load bitcode into the new debug info format by default.
+  if (LoadBitcodeIntoNewDbgInforFormat == cl::boolOrDefault::BOU_UNSET)
+    LoadBitcodeIntoNewDbgInforFormat = cl::boolOrDefault::BOU_TRUE;
 
   // RemoveDIs debug-info transition: tests may request that we /try/ to use the
   // new debug-info format.

Copy link
Contributor

@SLTozer SLTozer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems simple enough.

@OCHyams OCHyams merged commit b3f98df into llvm:main Mar 22, 2024
3 of 4 checks passed
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
…d llvm-lto2 (llvm#86271)

Directly load all bitcode into the new debug info format in `llvm-lto`
and `llvm-lto2`. This means that new-mode bitcode no longer round-trips
back to old-mode after parsing, and that old-mode bitcode gets
auto-upgraded to new-mode debug info (which is the current in-memory
default in LLVM).
@nikic
Copy link
Contributor

nikic commented Mar 23, 2024

OCHyams added a commit to OCHyams/llvm-project that referenced this pull request Mar 25, 2024
OCHyams added a commit to OCHyams/llvm-project that referenced this pull request Mar 25, 2024
…d llvm-lto2 (llvm#86271)

Directly load all bitcode into the new debug info format in `llvm-lto`
and `llvm-lto2`. This means that new-mode bitcode no longer round-trips
back to old-mode after parsing, and that old-mode bitcode gets
auto-upgraded to new-mode debug info (which is the current in-memory
default in LLVM).
@OCHyams
Copy link
Contributor Author

OCHyams commented Mar 25, 2024

Thanks @nikic, that wasn't expected, but after looking into it it looks like it's just because we're in a transitory state in the project. With this patch, we pay the cost of auto-upgrading to new-debug-info-mode because writing the new format to bitcode was disabled by default when this patch landed (due to a bot configuration issue -- see discussion starting here).

Using a branch for compiletimetracker (OCHyams perf/bitcodeloading) I reverted this patch, enabled bitcode writing by default, then relanded this patch. The stage1-ReleaseLTO-g config pasted below shows that it's not just the improvements from "enable bitcode writing by default" hiding the costs of this patch, but instead that combined together there's an improved reduction in compile time.

instructions:u

Commit     stage1-ReleaseLTO-g
2a4246645f 96062M (-0.87%) # This patch again
f53953c6d2 96904M (-0.24%) # Enable bitcode writing by default
ea3dff12f7 97140M (-0.68%) # Revert this patch
b3f98dff75 97806M          # This patch

I've re-landed the new-mode bitcode writing by default (cbbbab3) just now, so the transitory cost no longer exists, as we write and read in the new format (no auto-upgrade cost).

TL;DR, this was unexpected and is now understood; it was just temporary and the regression no longer exists as of cbbbab3

@jryans
Copy link
Member

jryans commented Mar 25, 2024

Using a branch for compiletimetracker (OCHyams perf/bitcodeloading) I reverted this patch, enabled bitcode writing by default, then relanded this patch. The stage1-ReleaseLTO-g config pasted below shows that it's not just the improvements from "enable bitcode writing by default" hiding the costs of this patch, but instead that combined together there's an improved reduction in compile time.

instructions:u

Commit     stage1-ReleaseLTO-g
2a4246645f 6062M (-0.87%) # This patch again
f53953c6d2 6904M (-0.24%) # Enable bitcode writing by default
ea3dff12f7 7140M (-0.68%) # Revert this patch
b3f98dff75 7806M          # This patch

Ah, at first I was having trouble matching those numbers with ones on the tracker, but it looks like you've removed the "9" digit at the front of all the instruction counts. 😉

Anyway, glad to see the compile time reduction! 😄

@OCHyams
Copy link
Contributor Author

OCHyams commented Mar 25, 2024

it looks like you've removed the "9" digit at the front of all the instruction counts

Oops! Edited my comment to fix that, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTO Link time optimization (regular/full LTO or ThinLTO)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants