[SYCL][NFC] Use unique_ptr for storing ModuleDesc. (#20358)

This patch resolves a warning about expensive copy in function
invocation which accepts ModuleDesc by value, thus taking ownership of
ModuleDesc. Now ownership is transferred into functions by using
unuqie_ptr. Also, the patch removes many usages of r-value references in
function's arguments because r-value reference supposes a ownership
transfer but does not oblige that.

Author: maksimsab

llvm/include/llvm/SYCLPostLink/ESIMDPostSplitProcessing.h

@@ -17,6 +17,8 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Error.h"
 
+#include <memory>
+
 namespace llvm {
 namespace sycl {
 
@@ -54,8 +56,8 @@ bool lowerESIMDConstructs(llvm::module_split::ModuleDesc &MD,
 /// \p Modified value indicates whether the Module has been modified.
 /// \p SplitOccurred value indicates whether split has occurred before or during
 /// function's invocation.
-Expected<SmallVector<module_split::ModuleDesc, 2>>
-handleESIMD(llvm::module_split::ModuleDesc MDesc,
+Expected<SmallVector<std::unique_ptr<module_split::ModuleDesc>, 2>>
+handleESIMD(std::unique_ptr<llvm::module_split::ModuleDesc> MDesc,
             const ESIMDProcessingOptions &Options, bool &Modified,
             bool &SplitOccurred);
 

llvm/include/llvm/SYCLPostLink/ModuleSplitter.h

@@ -141,7 +141,7 @@ class ModuleDesc {
   std::string Name = "";
   Properties Props;
 
-  ModuleDesc(std::unique_ptr<Module> &&M, StringRef Name = "TOP-LEVEL")
+  ModuleDesc(std::unique_ptr<Module> M, StringRef Name = "TOP-LEVEL")
       : M(std::move(M)), IsTopLevel(true), Name(Name) {
     // DeviceLib module doesn't include any entry point,it can be constructed
     // using ctor without any entry point related parameter.
@@ -153,13 +153,13 @@ class ModuleDesc {
     }
   }
 
-  ModuleDesc(std::unique_ptr<Module> &&M, EntryPointGroup &&EntryPoints,
+  ModuleDesc(std::unique_ptr<Module> M, EntryPointGroup &&EntryPoints,
              const Properties &Props)
       : M(std::move(M)), EntryPoints(std::move(EntryPoints)), Props(Props) {
     Name = this->EntryPoints.GroupId;
   }
 
-  ModuleDesc(std::unique_ptr<Module> &&M, const std::vector<std::string> &Names,
+  ModuleDesc(std::unique_ptr<Module> M, const std::vector<std::string> &Names,
              StringRef Name = "NoName")
       : M(std::move(M)), Name(Name) {
     rebuildEntryPoints(Names);
@@ -225,7 +225,7 @@ class ModuleDesc {
   bool isSpecConstantDefault() const;
   void setSpecConstantDefault(bool Value);
 
-  ModuleDesc clone() const;
+  std::unique_ptr<ModuleDesc> clone() const;
 
   std::string makeSymbolTable() const;
 
@@ -252,7 +252,7 @@ class ModuleDesc {
 // from input module that should be included in a split module.
 class ModuleSplitterBase {
 protected:
-  ModuleDesc Input;
+  std::unique_ptr<ModuleDesc> Input;
   EntryPointGroupVec Groups;
   bool AllowDeviceImageDependencies;
 
@@ -264,14 +264,15 @@ class ModuleSplitterBase {
     return Res;
   }
 
-  Module &getInputModule() { return Input.getModule(); }
+  Module &getInputModule() { return Input->getModule(); }
 
   std::unique_ptr<Module> releaseInputModule() {
-    return Input.releaseModulePtr();
+    return Input->releaseModulePtr();
   }
 
 public:
-  ModuleSplitterBase(ModuleDesc &&MD, EntryPointGroupVec &&GroupVec,
+  ModuleSplitterBase(std::unique_ptr<ModuleDesc> MD,
+                     EntryPointGroupVec &&GroupVec,
                      bool AllowDeviceImageDependencies)
       : Input(std::move(MD)), Groups(std::move(GroupVec)),
         AllowDeviceImageDependencies(AllowDeviceImageDependencies) {
@@ -288,7 +289,7 @@ class ModuleSplitterBase {
 
   // Gets next subsequence of entry points in an input module and provides split
   // submodule containing these entry points and their dependencies.
-  virtual ModuleDesc nextSplit() = 0;
+  virtual std::unique_ptr<ModuleDesc> nextSplit() = 0;
 
   // Returns a number of remaining modules, which can be split out using this
   // splitter. The value is reduced by 1 each time nextSplit is called.
@@ -298,13 +299,13 @@ class ModuleSplitterBase {
   bool hasMoreSplits() const { return remainingSplits() > 0; }
 };
 
-SmallVector<ModuleDesc, 2> splitByESIMD(ModuleDesc &&MD,
-                                        bool EmitOnlyKernelsAsEntryPoints,
-                                        bool AllowDeviceImageDependencies);
+SmallVector<std::unique_ptr<ModuleDesc>, 2>
+splitByESIMD(std::unique_ptr<ModuleDesc> MD, bool EmitOnlyKernelsAsEntryPoints,
+             bool AllowDeviceImageDependencies);
 
 std::unique_ptr<ModuleSplitterBase>
-getDeviceCodeSplitter(ModuleDesc &&MD, IRSplitMode Mode, bool IROutputOnly,
-                      bool EmitOnlyKernelsAsEntryPoints,
+getDeviceCodeSplitter(std::unique_ptr<ModuleDesc> MD, IRSplitMode Mode,
+                      bool IROutputOnly, bool EmitOnlyKernelsAsEntryPoints,
                       bool AllowDeviceImageDependencies);
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)

llvm/include/llvm/SYCLPostLink/SpecializationConstants.h

@@ -16,6 +16,7 @@
 #include "llvm/SYCLLowerIR/SpecConstants.h"
 #include "llvm/SYCLPostLink/ModuleSplitter.h"
 
+#include <memory>
 #include <optional>
 
 namespace llvm {
@@ -31,9 +32,10 @@ namespace sycl {
 /// \returns Boolean value indicating whether the lowering has changed the input
 /// modules.
 bool handleSpecializationConstants(
-    llvm::SmallVectorImpl<module_split::ModuleDesc> &MDs,
+    llvm::SmallVectorImpl<std::unique_ptr<module_split::ModuleDesc>> &MDs,
     std::optional<SpecConstantsPass::HandlingMode> Mode,
-    llvm::SmallVectorImpl<module_split::ModuleDesc> &NewModuleDescs,
+    llvm::SmallVectorImpl<std::unique_ptr<module_split::ModuleDesc>>
+        &NewModuleDescs,
     bool GenerateModuleDescWithDefaultSpecConsts);
 
 } // namespace sycl

llvm/lib/SYCLPostLink/ESIMDPostSplitProcessing.cpp

@@ -63,20 +63,22 @@ buildESIMDLoweringPipeline(const sycl::ESIMDProcessingOptions &Options) {
   return MPM;
 }
 
-Expected<ModuleDesc> linkModules(ModuleDesc MD1, ModuleDesc MD2) {
+Expected<std::unique_ptr<ModuleDesc>>
+linkModules(std::unique_ptr<ModuleDesc> MD1, std::unique_ptr<ModuleDesc> MD2) {
   std::vector<std::string> Names;
-  MD1.saveEntryPointNames(Names);
-  MD2.saveEntryPointNames(Names);
+  MD1->saveEntryPointNames(Names);
+  MD2->saveEntryPointNames(Names);
   bool LinkError =
-      llvm::Linker::linkModules(MD1.getModule(), MD2.releaseModulePtr());
+      llvm::Linker::linkModules(MD1->getModule(), MD2->releaseModulePtr());
 
   if (LinkError)
     return createStringError(
-        formatv("link failed. Module names: {0}, {1}", MD1.Name, MD2.Name));
+        formatv("link failed. Module names: {0}, {1}", MD1->Name, MD2->Name));
 
-  ModuleDesc Res(MD1.releaseModulePtr(), std::move(Names));
-  Res.assignMergedProperties(MD1, MD2);
-  Res.Name = (Twine("linked[") + MD1.Name + "," + MD2.Name + "]").str();
+  auto Res =
+      std::make_unique<ModuleDesc>(MD1->releaseModulePtr(), std::move(Names));
+  Res->assignMergedProperties(*MD1, *MD2);
+  Res->Name = (Twine("linked[") + MD1->Name + "," + MD2->Name + "]").str();
   return std::move(Res);
 }
 
@@ -110,11 +112,11 @@ bool sycl::lowerESIMDConstructs(ModuleDesc &MD,
   return !Res.areAllPreserved();
 }
 
-Expected<SmallVector<ModuleDesc, 2>>
-llvm::sycl::handleESIMD(ModuleDesc MDesc,
+Expected<SmallVector<std::unique_ptr<ModuleDesc>, 2>>
+llvm::sycl::handleESIMD(std::unique_ptr<ModuleDesc> MDesc,
                         const sycl::ESIMDProcessingOptions &Options,
                         bool &Modified, bool &SplitOccurred) {
-  SmallVector<ModuleDesc, 2> Result =
+  SmallVector<std::unique_ptr<ModuleDesc>, 2> Result =
       splitByESIMD(std::move(MDesc), Options.EmitOnlyKernelsAsEntryPoints,
                    Options.AllowDeviceImageDependencies);
 
@@ -123,32 +125,32 @@ llvm::sycl::handleESIMD(ModuleDesc MDesc,
 
   SplitOccurred |= Result.size() > 1;
 
-  for (ModuleDesc &MD : Result)
-    if (Options.LowerESIMD && MD.isESIMD())
-      Modified |= lowerESIMDConstructs(MD, Options);
+  for (std::unique_ptr<ModuleDesc> &MD : Result)
+    if (Options.LowerESIMD && MD->isESIMD())
+      Modified |= lowerESIMDConstructs(*MD, Options);
 
   if (Options.SplitESIMD || Result.size() == 1)
     return std::move(Result);
 
   // SYCL/ESIMD splitting is not requested, link back into single module.
-  int ESIMDInd = Result[0].isESIMD() ? 0 : 1;
+  int ESIMDInd = Result[0]->isESIMD() ? 0 : 1;
   int SYCLInd = 1 - ESIMDInd;
-  assert(Result[SYCLInd].isSYCL() &&
-         "Result[SYCLInd].isSYCL() expected to be true.");
+  assert(Result[SYCLInd]->isSYCL() &&
+         "Result[SYCLInd]->isSYCL() expected to be true.");
 
   // Make sure that no link conflicts occur.
-  Result[ESIMDInd].renameDuplicatesOf(Result[SYCLInd].getModule(), ".esimd");
+  Result[ESIMDInd]->renameDuplicatesOf(Result[SYCLInd]->getModule(), ".esimd");
   auto LinkedOrErr = linkModules(std::move(Result[0]), std::move(Result[1]));
   if (!LinkedOrErr)
     return LinkedOrErr.takeError();
 
-  ModuleDesc &Linked = *LinkedOrErr;
-  Linked.restoreLinkageOfDirectInvokeSimdTargets();
+  std::unique_ptr<ModuleDesc> &Linked = *LinkedOrErr;
+  Linked->restoreLinkageOfDirectInvokeSimdTargets();
   std::vector<std::string> Names;
-  Linked.saveEntryPointNames(Names);
+  Linked->saveEntryPointNames(Names);
   // Cleanup may remove some entry points, need to save/rebuild.
-  Linked.cleanup(Options.AllowDeviceImageDependencies);
-  Linked.rebuildEntryPoints(Names);
+  Linked->cleanup(Options.AllowDeviceImageDependencies);
+  Linked->rebuildEntryPoints(Names);
   Result.clear();
   Result.emplace_back(std::move(Linked));
   Modified = true;