Merge topic 'remove_unneeded_device_link_calls'

81b4d10d8f CUDA: More exhaustive checks to determine when to do device linking
3205c7c950 cmNinjaLinkLineDeviceComputer now lives in the correct source file

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !3320

Source/CMakeLists.txt

@@ -786,6 +786,8 @@ set(SRCS ${SRCS}
   cmNinjaUtilityTargetGenerator.h
   cmNinjaLinkLineComputer.cxx
   cmNinjaLinkLineComputer.h
+  cmNinjaLinkLineDeviceComputer.cxx
+  cmNinjaLinkLineDeviceComputer.h
   )
 
 # Temporary variable for tools targets

Source/cmLinkLineDeviceComputer.cxx

@@ -3,15 +3,20 @@
 
 #include "cmLinkLineDeviceComputer.h"
 
+#include <algorithm>
 #include <set>
 #include <sstream>
 #include <utility>
+#include <vector>
 
 #include "cmAlgorithms.h"
 #include "cmComputeLinkInformation.h"
 #include "cmGeneratorTarget.h"
-#include "cmGlobalNinjaGenerator.h"
+#include "cmLocalGenerator.h"
+#include "cmStateDirectory.h"
+#include "cmStateSnapshot.h"
 #include "cmStateTypes.h"
+#include "cmSystemTools.h"
 
 class cmOutputConverter;
 
@@ -41,6 +46,27 @@ static bool cmLinkItemValidForDevice(std::string const& item)
           cmHasLiteralPrefix(item, "--library"));
 }
 
+bool cmLinkLineDeviceComputer::ComputeRequiresDeviceLinking(
+  cmComputeLinkInformation& cli)
+{
+  // Determine if this item might requires device linking.
+  // For this we only consider targets
+  typedef cmComputeLinkInformation::ItemVector ItemVector;
+  ItemVector const& items = cli.GetItems();
+  std::string config = cli.GetConfig();
+  for (auto const& item : items) {
+    if (item.Target &&
+        item.Target->GetType() == cmStateEnums::STATIC_LIBRARY) {
+      if ((!item.Target->GetPropertyAsBool("CUDA_RESOLVE_DEVICE_SYMBOLS")) &&
+          item.Target->GetPropertyAsBool("CUDA_SEPARABLE_COMPILATION")) {
+        // this dependency requires us to device link it
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 std::string cmLinkLineDeviceComputer::ComputeLinkLibraries(
   cmComputeLinkInformation& cli, std::string const& stdLibString)
 {
@@ -63,17 +89,12 @@ std::string cmLinkLineDeviceComputer::ComputeLinkLibraries(
     }
 
     if (item.Target) {
-      bool skip = false;
-      switch (item.Target->GetType()) {
-        case cmStateEnums::MODULE_LIBRARY:
-        case cmStateEnums::INTERFACE_LIBRARY:
-          skip = true;
-          break;
-        case cmStateEnums::STATIC_LIBRARY:
-          skip = item.Target->GetPropertyAsBool("CUDA_RESOLVE_DEVICE_SYMBOLS");
-          break;
-        default:
-          break;
+      bool skip = true;
+      if (item.Target->GetType() == cmStateEnums::STATIC_LIBRARY) {
+        if ((!item.Target->GetPropertyAsBool("CUDA_RESOLVE_DEVICE_SYMBOLS")) &&
+            item.Target->GetPropertyAsBool("CUDA_SEPARABLE_COMPILATION")) {
+          skip = false;
+        }
       }
       if (skip) {
         continue;
@@ -118,16 +139,55 @@ std::string cmLinkLineDeviceComputer::GetLinkerLanguage(cmGeneratorTarget*,
   return "CUDA";
 }
 
-cmNinjaLinkLineDeviceComputer::cmNinjaLinkLineDeviceComputer(
-  cmOutputConverter* outputConverter, cmStateDirectory const& stateDir,
-  cmGlobalNinjaGenerator const* gg)
-  : cmLinkLineDeviceComputer(outputConverter, stateDir)
-  , GG(gg)
+bool requireDeviceLinking(cmGeneratorTarget& target, cmLocalGenerator& lg,
+                          const std::string& config)
 {
-}
 
-std::string cmNinjaLinkLineDeviceComputer::ConvertToLinkReference(
-  std::string const& lib) const
-{
-  return GG->ConvertToNinjaPath(lib);
+  if (target.GetType() == cmStateEnums::OBJECT_LIBRARY) {
+    return false;
+  }
+
+  if (const char* resolveDeviceSymbols =
+        target.GetProperty("CUDA_RESOLVE_DEVICE_SYMBOLS")) {
+    // If CUDA_RESOLVE_DEVICE_SYMBOLS has been explicitly set we need
+    // to honor the value no matter what it is.
+    return cmSystemTools::IsOn(resolveDeviceSymbols);
+  }
+
+  if (const char* separableCompilation =
+        target.GetProperty("CUDA_SEPARABLE_COMPILATION")) {
+    if (cmSystemTools::IsOn(separableCompilation)) {
+      bool doDeviceLinking = false;
+      switch (target.GetType()) {
+        case cmStateEnums::SHARED_LIBRARY:
+        case cmStateEnums::MODULE_LIBRARY:
+        case cmStateEnums::EXECUTABLE:
+          doDeviceLinking = true;
+          break;
+        default:
+          break;
+      }
+      return doDeviceLinking;
+    }
+  }
+
+  // Determine if we have any dependencies that require
+  // us to do a device link step
+  const std::string cuda_lang("CUDA");
+  cmGeneratorTarget::LinkClosure const* closure =
+    target.GetLinkClosure(config);
+
+  bool closureHasCUDA =
+    (std::find(closure->Languages.begin(), closure->Languages.end(),
+               cuda_lang) != closure->Languages.end());
+  if (closureHasCUDA) {
+    cmComputeLinkInformation* pcli = target.GetLinkInformation(config);
+    if (pcli) {
+      cmLinkLineDeviceComputer deviceLinkComputer(
+        &lg, lg.GetStateSnapshot().GetDirectory());
+      return deviceLinkComputer.ComputeRequiresDeviceLinking(*pcli);
+    }
+    return true;
+  }
+  return false;
 }

Source/cmLinkLineDeviceComputer.h

@@ -12,7 +12,7 @@
 
 class cmComputeLinkInformation;
 class cmGeneratorTarget;
-class cmGlobalNinjaGenerator;
+class cmLocalGenerator;
 class cmOutputConverter;
 class cmStateDirectory;
 
@@ -27,6 +27,8 @@ public:
   cmLinkLineDeviceComputer& operator=(cmLinkLineDeviceComputer const&) =
     delete;
 
+  bool ComputeRequiresDeviceLinking(cmComputeLinkInformation& cli);
+
   std::string ComputeLinkLibraries(cmComputeLinkInformation& cli,
                                    std::string const& stdLibString) override;
 
@@ -34,21 +36,7 @@ public:
                                 std::string const& config) override;
 };
 
-class cmNinjaLinkLineDeviceComputer : public cmLinkLineDeviceComputer
-{
-public:
-  cmNinjaLinkLineDeviceComputer(cmOutputConverter* outputConverter,
-                                cmStateDirectory const& stateDir,
-                                cmGlobalNinjaGenerator const* gg);
-
-  cmNinjaLinkLineDeviceComputer(cmNinjaLinkLineDeviceComputer const&) = delete;
-  cmNinjaLinkLineDeviceComputer& operator=(
-    cmNinjaLinkLineDeviceComputer const&) = delete;
-
-  std::string ConvertToLinkReference(std::string const& input) const override;
-
-private:
-  cmGlobalNinjaGenerator const* GG;
-};
+bool requireDeviceLinking(cmGeneratorTarget& target, cmLocalGenerator& lg,
+                          const std::string& config);
 
 #endif

Source/cmMakefileExecutableTargetGenerator.cxx

@@ -2,7 +2,6 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmMakefileExecutableTargetGenerator.h"
 
-#include <algorithm>
 #include <memory> // IWYU pragma: keep
 #include <set>
 #include <sstream>
@@ -87,20 +86,9 @@ void cmMakefileExecutableTargetGenerator::WriteDeviceExecutableRule(
     return;
   }
 
-  const std::string cuda_lang("CUDA");
-  cmGeneratorTarget::LinkClosure const* closure =
-    this->GeneratorTarget->GetLinkClosure(this->ConfigName);
-
-  const bool hasCUDA =
-    (std::find(closure->Languages.begin(), closure->Languages.end(),
-               cuda_lang) != closure->Languages.end());
-
-  bool doDeviceLinking = true;
-  if (const char* resolveDeviceSymbols =
-        this->GeneratorTarget->GetProperty("CUDA_RESOLVE_DEVICE_SYMBOLS")) {
-    doDeviceLinking = cmSystemTools::IsOn(resolveDeviceSymbols);
-  }
-  if (!hasCUDA || !doDeviceLinking) {
+  bool requiresDeviceLinking = requireDeviceLinking(
+    *this->GeneratorTarget, *this->LocalGenerator, this->ConfigName);
+  if (!requiresDeviceLinking) {
     return;
   }
 

Source/cmMakefileLibraryTargetGenerator.cxx

@@ -2,7 +2,6 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmMakefileLibraryTargetGenerator.h"
 
-#include <algorithm>
 #include <memory> // IWYU pragma: keep
 #include <set>
 #include <sstream>
@@ -124,20 +123,10 @@ void cmMakefileLibraryTargetGenerator::WriteObjectLibraryRules()
 
 void cmMakefileLibraryTargetGenerator::WriteStaticLibraryRules()
 {
-  const std::string cuda_lang("CUDA");
-  cmGeneratorTarget::LinkClosure const* closure =
-    this->GeneratorTarget->GetLinkClosure(this->ConfigName);
 
-  const bool hasCUDA =
-    (std::find(closure->Languages.begin(), closure->Languages.end(),
-               cuda_lang) != closure->Languages.end());
-
-  bool doDeviceLinking = false;
-  if (const char* resolveDeviceSymbols =
-        this->GeneratorTarget->GetProperty("CUDA_RESOLVE_DEVICE_SYMBOLS")) {
-    doDeviceLinking = cmSystemTools::IsOn(resolveDeviceSymbols);
-  }
-  if (hasCUDA && doDeviceLinking) {
+  bool requiresDeviceLinking = requireDeviceLinking(
+    *this->GeneratorTarget, *this->LocalGenerator, this->ConfigName);
+  if (requiresDeviceLinking) {
     std::string linkRuleVar = "CMAKE_CUDA_DEVICE_LINK_LIBRARY";
     this->WriteDeviceLibraryRules(linkRuleVar, false);
   }
@@ -163,19 +152,9 @@ void cmMakefileLibraryTargetGenerator::WriteSharedLibraryRules(bool relink)
   }
 
   if (!relink) {
-    const std::string cuda_lang("CUDA");
-    cmGeneratorTarget::LinkClosure const* closure =
-      this->GeneratorTarget->GetLinkClosure(this->ConfigName);
-
-    const bool hasCUDA =
-      (std::find(closure->Languages.begin(), closure->Languages.end(),
-                 cuda_lang) != closure->Languages.end());
-    bool doDeviceLinking = true;
-    if (const char* resolveDeviceSymbols =
-          this->GeneratorTarget->GetProperty("CUDA_RESOLVE_DEVICE_SYMBOLS")) {
-      doDeviceLinking = cmSystemTools::IsOn(resolveDeviceSymbols);
-    }
-    if (hasCUDA && doDeviceLinking) {
+    bool requiresDeviceLinking = requireDeviceLinking(
+      *this->GeneratorTarget, *this->LocalGenerator, this->ConfigName);
+    if (requiresDeviceLinking) {
       std::string linkRuleVar = "CMAKE_CUDA_DEVICE_LINK_LIBRARY";
       this->WriteDeviceLibraryRules(linkRuleVar, relink);
     }
@@ -209,19 +188,9 @@ void cmMakefileLibraryTargetGenerator::WriteModuleLibraryRules(bool relink)
 {
 
   if (!relink) {
-    const std::string cuda_lang("CUDA");
-    cmGeneratorTarget::LinkClosure const* closure =
-      this->GeneratorTarget->GetLinkClosure(this->ConfigName);
-
-    const bool hasCUDA =
-      (std::find(closure->Languages.begin(), closure->Languages.end(),
-                 cuda_lang) != closure->Languages.end());
-    bool doDeviceLinking = true;
-    if (const char* resolveDeviceSymbols =
-          this->GeneratorTarget->GetProperty("CUDA_RESOLVE_DEVICE_SYMBOLS")) {
-      doDeviceLinking = cmSystemTools::IsOn(resolveDeviceSymbols);
-    }
-    if (hasCUDA && doDeviceLinking) {
+    bool requiresDeviceLinking = requireDeviceLinking(
+      *this->GeneratorTarget, *this->LocalGenerator, this->ConfigName);
+    if (requiresDeviceLinking) {
       std::string linkRuleVar = "CMAKE_CUDA_DEVICE_LINK_LIBRARY";
       this->WriteDeviceLibraryRules(linkRuleVar, relink);
     }

Source/cmNinjaLinkLineDeviceComputer.cxx

@@ -0,0 +1,20 @@
+/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
+   file Copyright.txt or https://cmake.org/licensing for details.  */
+
+#include "cmNinjaLinkLineDeviceComputer.h"
+
+#include "cmGlobalNinjaGenerator.h"
+
+cmNinjaLinkLineDeviceComputer::cmNinjaLinkLineDeviceComputer(
+  cmOutputConverter* outputConverter, cmStateDirectory const& stateDir,
+  cmGlobalNinjaGenerator const* gg)
+  : cmLinkLineDeviceComputer(outputConverter, stateDir)
+  , GG(gg)
+{
+}
+
+std::string cmNinjaLinkLineDeviceComputer::ConvertToLinkReference(
+  std::string const& lib) const
+{
+  return GG->ConvertToNinjaPath(lib);
+}

Source/cmNinjaLinkLineDeviceComputer.h

@@ -0,0 +1,34 @@
+/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
+   file Copyright.txt or https://cmake.org/licensing for details.  */
+
+#ifndef cmNinjaLinkLineDeviceComputer_h
+#define cmNinjaLinkLineDeviceComputer_h
+
+#include "cmConfigure.h" // IWYU pragma: keep
+
+#include <string>
+
+#include "cmLinkLineDeviceComputer.h"
+
+class cmGlobalNinjaGenerator;
+class cmOutputConverter;
+class cmStateDirectory;
+
+class cmNinjaLinkLineDeviceComputer : public cmLinkLineDeviceComputer
+{
+public:
+  cmNinjaLinkLineDeviceComputer(cmOutputConverter* outputConverter,
+                                cmStateDirectory const& stateDir,
+                                cmGlobalNinjaGenerator const* gg);
+
+  cmNinjaLinkLineDeviceComputer(cmNinjaLinkLineDeviceComputer const&) = delete;
+  cmNinjaLinkLineDeviceComputer& operator=(
+    cmNinjaLinkLineDeviceComputer const&) = delete;
+
+  std::string ConvertToLinkReference(std::string const& input) const override;
+
+private:
+  cmGlobalNinjaGenerator const* GG;
+};
+
+#endif

Source/cmNinjaNormalTargetGenerator.cxx

@@ -22,6 +22,7 @@
 #include "cmLocalGenerator.h"
 #include "cmLocalNinjaGenerator.h"
 #include "cmMakefile.h"
+#include "cmNinjaLinkLineDeviceComputer.h"
 #include "cmNinjaTypes.h"
 #include "cmOSXBundleGenerator.h"
 #include "cmOutputConverter.h"
@@ -574,32 +575,9 @@ void cmNinjaNormalTargetGenerator::WriteDeviceLinkStatement()
 
   cmGeneratorTarget& genTarget = *this->GetGeneratorTarget();
 
-  // determine if we need to do any device linking for this target
-  const std::string cuda_lang("CUDA");
-  cmGeneratorTarget::LinkClosure const* closure =
-    genTarget.GetLinkClosure(this->GetConfigName());
-
-  const bool hasCUDA =
-    (std::find(closure->Languages.begin(), closure->Languages.end(),
-               cuda_lang) != closure->Languages.end());
-
-  bool doDeviceLinking = false;
-  if (const char* resolveDeviceSymbols =
-        genTarget.GetProperty("CUDA_RESOLVE_DEVICE_SYMBOLS")) {
-    doDeviceLinking = cmSystemTools::IsOn(resolveDeviceSymbols);
-  } else {
-    switch (genTarget.GetType()) {
-      case cmStateEnums::SHARED_LIBRARY:
-      case cmStateEnums::MODULE_LIBRARY:
-      case cmStateEnums::EXECUTABLE:
-        doDeviceLinking = true;
-        break;
-      default:
-        break;
-    }
-  }
-
-  if (!(doDeviceLinking && hasCUDA)) {
+  bool requiresDeviceLinking = requireDeviceLinking(
+    *this->GeneratorTarget, *this->GetLocalGenerator(), this->ConfigName);
+  if (!requiresDeviceLinking) {
     return;
   }
 

Source/cmVisualStudio10TargetGenerator.cxx

@@ -10,6 +10,7 @@
 #include "cmGeneratorExpression.h"
 #include "cmGeneratorTarget.h"
 #include "cmGlobalVisualStudio10Generator.h"
+#include "cmLinkLineDeviceComputer.h"
 #include "cmLocalVisualStudio10Generator.h"
 #include "cmMakefile.h"
 #include "cmSourceFile.h"
@@ -3007,21 +3008,8 @@ bool cmVisualStudio10TargetGenerator::ComputeCudaLinkOptions(
   Options& cudaLinkOptions = *pOptions;
 
   // Determine if we need to do a device link
-  bool doDeviceLinking = false;
-  if (const char* resolveDeviceSymbols =
-        this->GeneratorTarget->GetProperty("CUDA_RESOLVE_DEVICE_SYMBOLS")) {
-    doDeviceLinking = cmSystemTools::IsOn(resolveDeviceSymbols);
-  } else {
-    switch (this->GeneratorTarget->GetType()) {
-      case cmStateEnums::SHARED_LIBRARY:
-      case cmStateEnums::MODULE_LIBRARY:
-      case cmStateEnums::EXECUTABLE:
-        doDeviceLinking = true;
-        break;
-      default:
-        break;
-    }
-  }
+  bool doDeviceLinking = requireDeviceLinking(
+    *this->GeneratorTarget, *this->LocalGenerator, configName);
 
   cudaLinkOptions.AddFlag("PerformDeviceLink",
                           doDeviceLinking ? "true" : "false");

bootstrap

@@ -359,6 +359,7 @@ CMAKE_CXX_SOURCES="\
   cmLinkDirectoriesCommand \
   cmLinkItem \
   cmLinkLineComputer \
+  cmLinkLineDeviceComputer \
   cmListCommand \
   cmListFileCache \
   cmLocalCommonGenerator \