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

[LTO] Allow target-specific module splittting #83128

Merged
merged 2 commits into from
Apr 22, 2024

Conversation

Pierre-vh
Copy link
Contributor

@Pierre-vh Pierre-vh commented Feb 27, 2024

Allow targets to implement custom module splitting logic for --lto-partitions, see #89245

https://discourse.llvm.org/t/rfc-lto-target-specific-module-splittting/77252

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

llvmbot commented Feb 27, 2024

@llvm/pr-subscribers-lto

Author: Pierre van Houtryve (Pierre-vh)

Changes

Allow targets to implement custom module splitting logic for --lto-partitions.


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

4 Files Affected:

  • (modified) llvm/include/llvm/Target/TargetMachine.h (+10)
  • (modified) llvm/lib/LTO/LTOBackend.cpp (+8-4)
  • (modified) llvm/tools/llvm-split/CMakeLists.txt (+7)
  • (modified) llvm/tools/llvm-split/llvm-split.cpp (+77-25)
diff --git a/llvm/include/llvm/Target/TargetMachine.h b/llvm/include/llvm/Target/TargetMachine.h
index d7ce088cad49f6..43342c3e474615 100644
--- a/llvm/include/llvm/Target/TargetMachine.h
+++ b/llvm/include/llvm/Target/TargetMachine.h
@@ -418,6 +418,16 @@ class TargetMachine {
   virtual unsigned getAddressSpaceForPseudoSourceKind(unsigned Kind) const {
     return 0;
   }
+
+  /// Entry point for module splitting. Targets can implement custom module
+  /// splitting logic, mainly used by LTO for --lto-partitions.
+  ///
+  /// \returns `true` if the module was split, `false` otherwise. When  `false` is returned, it is assumed that \p ModuleCallback has never been called and \p M has not been modified.
+  virtual bool splitModule(
+      Module &M, unsigned NumParts,
+      function_ref<void(std::unique_ptr<Module> MPart)> ModuleCallback) const {
+    return false;
+  }
 };
 
 /// This class describes a target machine that is implemented with the LLVM
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 6cfe67779b1a7d..494a8b0798900b 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -436,8 +436,7 @@ static void splitCodeGen(const Config &C, TargetMachine *TM,
   unsigned ThreadCount = 0;
   const Target *T = &TM->getTarget();
 
-  SplitModule(
-      Mod, ParallelCodeGenParallelismLevel,
+  const auto HandleModulePartition =
       [&](std::unique_ptr<Module> MPart) {
         // We want to clone the module in a new context to multi-thread the
         // codegen. We do it by serializing partition modules to bitcode
@@ -469,8 +468,13 @@ static void splitCodeGen(const Config &C, TargetMachine *TM,
             // Pass BC using std::move to ensure that it get moved rather than
             // copied into the thread's context.
             std::move(BC), ThreadCount++);
-      },
-      false);
+      };
+
+  // Try target-specific module splitting first, then fallback to the default.
+  if (!TM->splitModule(Mod, ParallelCodeGenParallelismLevel,
+                                HandleModulePartition)) {
+    SplitModule(Mod, ParallelCodeGenParallelismLevel, HandleModulePartition, false);
+  }
 
   // Because the inner lambda (which runs in a worker thread) captures our local
   // variables, we need to wait for the worker threads to terminate before we
diff --git a/llvm/tools/llvm-split/CMakeLists.txt b/llvm/tools/llvm-split/CMakeLists.txt
index 52eedeb9f53f32..0579298462d113 100644
--- a/llvm/tools/llvm-split/CMakeLists.txt
+++ b/llvm/tools/llvm-split/CMakeLists.txt
@@ -1,9 +1,16 @@
 set(LLVM_LINK_COMPONENTS
+  AllTargetsAsmParsers
+  AllTargetsCodeGens
+  AllTargetsDescs
+  AllTargetsInfos
   TransformUtils
   BitWriter
+  CodeGen
   Core
   IRReader
+  MC
   Support
+  Target
   )
 
 add_llvm_tool(llvm-split
diff --git a/llvm/tools/llvm-split/llvm-split.cpp b/llvm/tools/llvm-split/llvm-split.cpp
index c6e20e0373c717..f463135c67504f 100644
--- a/llvm/tools/llvm-split/llvm-split.cpp
+++ b/llvm/tools/llvm-split/llvm-split.cpp
@@ -1,4 +1,4 @@
-//===-- llvm-split: command line tool for testing module splitter ---------===//
+//===-- llvm-split: command line tool for testing module splitting --------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,7 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This program can be used to test the llvm::SplitModule function.
+// This program can be used to test the llvm::SplitModule and
+// TargetMachine::splitModule functions.
 //
 //===----------------------------------------------------------------------===//
 
@@ -15,12 +16,17 @@
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/IRReader/IRReader.h"
+#include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/ToolOutputFile.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetMachine.h"
+#include "llvm/TargetParser/Triple.h"
 #include "llvm/Transforms/Utils/SplitModule.h"
 
 using namespace llvm;
@@ -47,12 +53,45 @@ static cl::opt<bool>
                    cl::desc("Split without externalizing locals"),
                    cl::cat(SplitCategory));
 
+static cl::opt<std::string>
+    MTarget("mtarget",
+            cl::desc("Target triple. When present, a TargetMachine is created "
+                     "and TargetMachine::splitModule is used instead of the "
+                     "common SplitModule logic."),
+            cl::value_desc("triple"), cl::cat(SplitCategory));
+
+static cl::opt<std::string>
+    MCPU("mcpu", cl::desc("Target CPU, ignored if -mtarget is not used"),
+         cl::value_desc("cpu"), cl::cat(SplitCategory));
+
 int main(int argc, char **argv) {
+  InitLLVM X(argc, argv);
+
+  // NOTE: If mtarget is not present we could avoid initializing targets to save
+  // time, but this is a testing tool and it's likely not worth the added
+  // complexity.
+  InitializeAllTargets();
+  InitializeAllTargetMCs();
+
   LLVMContext Context;
   SMDiagnostic Err;
   cl::HideUnrelatedOptions({&SplitCategory, &getColorCategory()});
   cl::ParseCommandLineOptions(argc, argv, "LLVM module splitter\n");
 
+  TargetMachine *TM = nullptr;
+  if (!MTarget.empty()) {
+    std::string Error;
+    const Target *T = TargetRegistry::lookupTarget(MTarget, Error);
+    if (!T) {
+      errs() << "unknown target '" << MTarget << "': " << Error << "\n";
+      return 1;
+    }
+
+    TargetOptions Options;
+    TM = T->createTargetMachine(MTarget, MCPU, /*FS*/ "", Options, std::nullopt,
+                                std::nullopt);
+  }
+
   std::unique_ptr<Module> M = parseIRFile(InputFilename, Err, Context);
 
   if (!M) {
@@ -61,28 +100,41 @@ int main(int argc, char **argv) {
   }
 
   unsigned I = 0;
-  SplitModule(
-      *M, NumOutputs,
-      [&](std::unique_ptr<Module> MPart) {
-        std::error_code EC;
-        std::unique_ptr<ToolOutputFile> Out(new ToolOutputFile(
-            OutputFilename + utostr(I++), EC, sys::fs::OF_None));
-        if (EC) {
-          errs() << EC.message() << '\n';
-          exit(1);
-        }
-
-        if (verifyModule(*MPart, &errs())) {
-          errs() << "Broken module!\n";
-          exit(1);
-        }
-
-        WriteBitcodeToFile(*MPart, Out->os());
-
-        // Declare success.
-        Out->keep();
-      },
-      PreserveLocals);
+  const auto HandleModulePart = [&](std::unique_ptr<Module> MPart) {
+    std::error_code EC;
+    std::unique_ptr<ToolOutputFile> Out(
+        new ToolOutputFile(OutputFilename + utostr(I++), EC, sys::fs::OF_None));
+    if (EC) {
+      errs() << EC.message() << '\n';
+      exit(1);
+    }
+
+    if (verifyModule(*MPart, &errs())) {
+      errs() << "Broken module!\n";
+      exit(1);
+    }
+
+    WriteBitcodeToFile(*MPart, Out->os());
+
+    // Declare success.
+    Out->keep();
+  };
+
+  if (!TM) {
+    SplitModule(*M, NumOutputs, HandleModulePart, PreserveLocals);
+    return 0;
+  }
+
+  if (PreserveLocals) {
+    errs() << "warning: -preserve-locals has no effect when using "
+              "TargetMachine::splitModule\n";
+  }
+
+  if (!TM->splitModule(*M, NumOutputs, HandleModulePart)) {
+    errs() << "error: '" << MTarget
+           << "' does not implement TargetMachine::splitModule\n";
+    return 1;
+  }
 
   return 0;
 }

Copy link

github-actions bot commented Feb 27, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@teresajohnson teresajohnson left a comment

Choose a reason for hiding this comment

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

@pcc who implemented split LTO code gen

I am not a user of this functionality, but it seems reasonable to me. It would be good to see the companion PR with the use case as well when reviewing.

"TargetMachine::splitModule\n";
}

if (!TM->splitModule(*M, NumOutputs, HandleModulePart)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be structured like in LTOBackend.cpp, i.e. where it falls back to the default SplitModule if this returns false, instead of giving an error here? I guess the tool is designed to require use of splitModule if a target is given, but allowing it to fall back would allow adding an llvm-split based test with this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Allow targets to implement custom module splitting logic for
--lto-partitions.
@Pierre-vh
Copy link
Contributor Author

#89245 is a target-specific implementation of module splitting, which uses this system.

Copy link
Contributor

@teresajohnson teresajohnson left a comment

Choose a reason for hiding this comment

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

If this is ready for a full review please remove "[RFC]" from the title. Also, can you add a basic llvm-split based test? Which I guess should just ensure that the fallback splitting works since there is no target implementation in this PR.

llvm/tools/llvm-split/llvm-split.cpp Outdated Show resolved Hide resolved
@Pierre-vh Pierre-vh changed the title [RFC][LTO] Allow target-specific module splittting [LTO] Allow target-specific module splittting Apr 19, 2024
@Pierre-vh Pierre-vh merged commit e86ebe4 into llvm:main Apr 22, 2024
4 checks passed
@Pierre-vh Pierre-vh deleted the lto-splitmodule-rfc branch April 22, 2024 06:59
@dtcxzyw
Copy link
Member

dtcxzyw commented Apr 22, 2024

@Pierre-vh This patch breaks our downstream build (with GCC 10.2.1):

FAILED: bin/llvm-split 
: && /usr/bin/c++ -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -w -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -Wl,-rpath-link,/home/dtcxzyw/llvm-build/./lib  -Wl,--gc-sections tools/llvm-split/CMakeFiles/llvm-split.dir/llvm-split.cpp.o -o bin/llvm-split  -Wl,-rpath,"\$ORIGIN/../lib:/home/dtcxzyw/llvm-build/lib:"  lib/libLLVMX86AsmParser.so.19.0git  lib/libLLVMRISCVAsmParser.so.19.0git  lib/libLLVMX86CodeGen.so.19.0git  lib/libLLVMRISCVCodeGen.so.19.0git  lib/libLLVMX86Desc.so.19.0git  lib/libLLVMRISCVDesc.so.19.0git  lib/libLLVMX86Info.so.19.0git  lib/libLLVMRISCVInfo.so.19.0git  -lpthread  lib/libLLVMCodeGen.so.19.0git  lib/libLLVMTarget.so.19.0git  lib/libLLVMBitWriter.so.19.0git  lib/libLLVMTransformUtils.so.19.0git  lib/libLLVMIRReader.so.19.0git  lib/libLLVMMC.so.19.0git  lib/libLLVMCore.so.19.0git  lib/libLLVMSupport.so.19.0git  -Wl,-rpath-link,/home/dtcxzyw/llvm-build/lib && :
/usr/bin/ld: tools/llvm-split/CMakeFiles/llvm-split.dir/llvm-split.cpp.o: undefined reference to symbol '_ZN4llvm6TripleC1ERKNS_5TwineE'
/usr/bin/ld: /home/dtcxzyw/llvm-build/./lib/libLLVMTargetParser.so.19.0git: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

@Pierre-vh
Copy link
Contributor Author

@Pierre-vh This patch breaks our downstream build (with GCC 10.2.1):

FAILED: bin/llvm-split 
: && /usr/bin/c++ -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -w -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -Wl,-rpath-link,/home/dtcxzyw/llvm-build/./lib  -Wl,--gc-sections tools/llvm-split/CMakeFiles/llvm-split.dir/llvm-split.cpp.o -o bin/llvm-split  -Wl,-rpath,"\$ORIGIN/../lib:/home/dtcxzyw/llvm-build/lib:"  lib/libLLVMX86AsmParser.so.19.0git  lib/libLLVMRISCVAsmParser.so.19.0git  lib/libLLVMX86CodeGen.so.19.0git  lib/libLLVMRISCVCodeGen.so.19.0git  lib/libLLVMX86Desc.so.19.0git  lib/libLLVMRISCVDesc.so.19.0git  lib/libLLVMX86Info.so.19.0git  lib/libLLVMRISCVInfo.so.19.0git  -lpthread  lib/libLLVMCodeGen.so.19.0git  lib/libLLVMTarget.so.19.0git  lib/libLLVMBitWriter.so.19.0git  lib/libLLVMTransformUtils.so.19.0git  lib/libLLVMIRReader.so.19.0git  lib/libLLVMMC.so.19.0git  lib/libLLVMCore.so.19.0git  lib/libLLVMSupport.so.19.0git  -Wl,-rpath-link,/home/dtcxzyw/llvm-build/lib && :
/usr/bin/ld: tools/llvm-split/CMakeFiles/llvm-split.dir/llvm-split.cpp.o: undefined reference to symbol '_ZN4llvm6TripleC1ERKNS_5TwineE'
/usr/bin/ld: /home/dtcxzyw/llvm-build/./lib/libLLVMTargetParser.so.19.0git: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

adb2547

@dtcxzyw
Copy link
Member

dtcxzyw commented Apr 22, 2024

@Pierre-vh This patch breaks our downstream build (with GCC 10.2.1):

FAILED: bin/llvm-split 
: && /usr/bin/c++ -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -w -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -Wl,-rpath-link,/home/dtcxzyw/llvm-build/./lib  -Wl,--gc-sections tools/llvm-split/CMakeFiles/llvm-split.dir/llvm-split.cpp.o -o bin/llvm-split  -Wl,-rpath,"\$ORIGIN/../lib:/home/dtcxzyw/llvm-build/lib:"  lib/libLLVMX86AsmParser.so.19.0git  lib/libLLVMRISCVAsmParser.so.19.0git  lib/libLLVMX86CodeGen.so.19.0git  lib/libLLVMRISCVCodeGen.so.19.0git  lib/libLLVMX86Desc.so.19.0git  lib/libLLVMRISCVDesc.so.19.0git  lib/libLLVMX86Info.so.19.0git  lib/libLLVMRISCVInfo.so.19.0git  -lpthread  lib/libLLVMCodeGen.so.19.0git  lib/libLLVMTarget.so.19.0git  lib/libLLVMBitWriter.so.19.0git  lib/libLLVMTransformUtils.so.19.0git  lib/libLLVMIRReader.so.19.0git  lib/libLLVMMC.so.19.0git  lib/libLLVMCore.so.19.0git  lib/libLLVMSupport.so.19.0git  -Wl,-rpath-link,/home/dtcxzyw/llvm-build/lib && :
/usr/bin/ld: tools/llvm-split/CMakeFiles/llvm-split.dir/llvm-split.cpp.o: undefined reference to symbol '_ZN4llvm6TripleC1ERKNS_5TwineE'
/usr/bin/ld: /home/dtcxzyw/llvm-build/./lib/libLLVMTargetParser.so.19.0git: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

adb2547

Thank you!

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request May 28, 2024
Allow targets to implement custom module splitting logic for
--lto-partitions, see llvm#89245

https: //discourse.llvm.org/t/rfc-lto-target-specific-module-splittting/77252
Change-Id: I22abc7f37988e839eb7b06fbd1a0c69dfef1af28
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

4 participants