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

[lld][COFF]Expose UnifiedLTO and PassPipeline Options #69904

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Naville
Copy link
Contributor

@Naville Naville commented Oct 23, 2023

ab9b3c8 added UnifiedLTO support and related arguments to lld-ELF.
This patch attempts to do the same with lld-COFF.

I'm not familiar with LLVM's TableGen based OptParser so a second pair of eyes might be required

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 23, 2023

@llvm/pr-subscribers-lld
@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld-coff

Author: Zhang (Naville)

Changes

ab9b3c8 added UnifiedLTO support and related arguments to lld-ELF.
This patch attempts to do the same with lld-COFF.

I'm not familiar with LLVM's TableGen based OptParser so a second pair of eyes might be required


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

4 Files Affected:

  • (modified) lld/COFF/Config.h (+3)
  • (modified) lld/COFF/Driver.cpp (+14)
  • (modified) lld/COFF/LTO.cpp (+1-1)
  • (modified) lld/COFF/Options.td (+3)
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 1c338cc63fa87d2..186fb17745e21e2 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -17,6 +17,7 @@
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/LTO/LTO.h"
 #include <cstdint>
 #include <map>
 #include <set>
@@ -27,6 +28,7 @@ namespace lld::coff {
 using llvm::COFF::IMAGE_FILE_MACHINE_UNKNOWN;
 using llvm::COFF::WindowsSubsystem;
 using llvm::StringRef;
+using llvm::lto::LTO;
 class DefinedAbsolute;
 class StringChunk;
 class Symbol;
@@ -317,6 +319,7 @@ struct Configuration {
   bool writeCheckSum = false;
   EmitKind emit = EmitKind::Obj;
   bool allowDuplicateWeak = false;
+  LTO::LTOKind ltoKind;
 };
 
 } // namespace lld::coff
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 5613c2e6993a5af..2b127de80e89531 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -59,6 +59,7 @@ using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::COFF;
 using namespace llvm::sys;
+using namespace llvm::lto;
 
 namespace lld::coff {
 
@@ -1886,6 +1887,19 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
         error("/opt: unknown option: " + s);
     }
   }
+  // Handle /lto
+  config->ltoKind = LTO::LTOK_Default;
+  if (auto *arg = args.getLastArg(OPT_lto)) {
+    StringRef s = arg->getValue();
+    if (s == "thin")
+      config->ltoKind = LTO::LTOK_UnifiedThin;
+    else if (s == "full")
+      config->ltoKind = LTO::LTOK_UnifiedRegular;
+    else if (s == "default")
+      config->ltoKind = LTO::LTOK_Default;
+    else
+      error("unknown LTO mode: " + s);
+  }
 
   if (!icfLevel)
     icfLevel = doGC ? ICFLevel::All : ICFLevel::None;
diff --git a/lld/COFF/LTO.cpp b/lld/COFF/LTO.cpp
index 7df931911213672..c559449ae3b4ef2 100644
--- a/lld/COFF/LTO.cpp
+++ b/lld/COFF/LTO.cpp
@@ -127,7 +127,7 @@ BitcodeCompiler::BitcodeCompiler(COFFLinkerContext &c) : ctx(c) {
   }
 
   ltoObj = std::make_unique<lto::LTO>(createConfig(), backend,
-                                      ctx.config.ltoPartitions);
+                                      ctx.config.ltoPartitions,ctx.config.ltoKind);
 }
 
 BitcodeCompiler::~BitcodeCompiler() = default;
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index 977657a433dc581..6eec0cf44bb0c4f 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -309,6 +309,9 @@ def print_search_paths : F<"print-search-paths">;
 def show_timing : F<"time">;
 def summary : F<"summary">;
 
+// Flags for LTO
+def lto: P<"lto=","Set LTO backend">,MetaVarName<"[full,thin]">;
+
 //==============================================================================
 // The flags below do nothing. They are defined only for link.exe compatibility.
 //==============================================================================

@github-actions
Copy link

github-actions bot commented Oct 23, 2023

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff ac24238002076e36b2d33d18d1bf47a9de59fab4 79d318ca0deb68d8392b94ebe9fa2df90348f41b -- lld/COFF/Config.h lld/COFF/Driver.cpp lld/COFF/LTO.cpp
View the diff from clang-format here.
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 8e86d5493b3d..065c413d7a3c 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -102,10 +102,10 @@ enum class ICFLevel {
         // behavior.
 };
 
-enum class LTOKind{
-  Default, // Any LTO mode without Unified LTO. The default mode.
+enum class LTOKind {
+  Default,        // Any LTO mode without Unified LTO. The default mode.
   UnifiedRegular, // Regular LTO, with Unified LTO enabled.
-  UnifiedThin // ThinLTO, with Unified LTO enabled.
+  UnifiedThin     // ThinLTO, with Unified LTO enabled.
 };
 
 // Global configuration.

@mstorsjo
Copy link
Member

This needs a testcase added to test the new option.

lld/COFF/Config.h Outdated Show resolved Hide resolved
@Naville
Copy link
Contributor Author

Naville commented Oct 24, 2023

@rnk Can you kindly review if the new approach that doesn't involve reincluding LTO.h is fine?
If so I'll start adding tests

@Naville Naville changed the title [lld][COFF]Expose UnifiedLTO options [lld][COFF]Expose UnifiedLTO and PassPipeline Options Oct 24, 2023
@Naville
Copy link
Contributor Author

Naville commented Oct 24, 2023

CrossPosting from Discord:

  • I'm not sure how to convert my cpp reproducible to an LTO test case. Kinda need some help on that part.
  • Should I also add the remaining LTO options too?

Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good, but I have some minor nits.

lld/COFF/LTO.cpp Outdated Show resolved Hide resolved
@rnk
Copy link
Collaborator

rnk commented Oct 24, 2023

Sorry I missed this question

I'm not sure how to convert my cpp reproducible to an LTO test case. Kinda need some help on that part.

I guess I would look at the ELF test case and try to remodel it for COFF:
ab9b3c8#diff-71ce44d5f2d7691e5ce7dd5907ddec5a2a8092b1743d8d65ad33a92fece521a1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants