[NFC][Clang][Driver][AMDGPU] Avoid temporary copies of std::string by using Twine and StringRef

[tra]

LGTM in principle.

That said, I don't think StringRef buys us anything here as none of this code is in the hot path. It also comes with the downside that now one has to worry about lifetimes of the strings we refer to. Things do look OK in this case, but it would be good if it could be confirmed with sanitizer.
In code that's not performance critical I personally prefer passing std::string by value and let compiler optimize them away.

[jmmartinez]

@tra Thanks for the review! I double checked with ASAN and there were no issues.

clang/lib/Driver/ToolChains/AMDGPU.cpp

void ROCMToolChain::addClangTargetOptions(
 
   if (DriverArgs.hasArg(options::OPT_nogpulib))
     return;
 
   // Get the device name and canonicalize it
   const StringRef GpuArch = getGPUArch(DriverArgs);
   auto Kind = llvm::AMDGPU::parseArchAMDGCN(GpuArch);
   const StringRef CanonArch = llvm::AMDGPU::getArchNameAMDGCN(Kind);
-  std::string LibDeviceFile = RocmInstallation.getLibDeviceFile(CanonArch);
+  StringRef LibDeviceFile = RocmInstallation.getLibDeviceFile(CanonArch);
   auto ABIVer = DeviceLibABIVersion::fromCodeObjectVersion(
       getAMDGPUCodeObjectVersion(getDriver(), DriverArgs));
   if (!RocmInstallation.checkCommonBitcodeLibs(CanonArch, LibDeviceFile,
                                                ABIVer))
     return;
 
   bool Wave64 = isWave64(DriverArgs, Kind);
 

 
 llvm::SmallVector<std::string, 12>
 ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
                                        const std::string &GPUArch,
                                        bool isOpenMP) const {
   auto Kind = llvm::AMDGPU::parseArchAMDGCN(GPUArch);
   const StringRef CanonArch = llvm::AMDGPU::getArchNameAMDGCN(Kind);
 
-  std::string LibDeviceFile = RocmInstallation.getLibDeviceFile(CanonArch);
+  StringRef LibDeviceFile = RocmInstallation.getLibDeviceFile(CanonArch);
   auto ABIVer = DeviceLibABIVersion::fromCodeObjectVersion(
       getAMDGPUCodeObjectVersion(getDriver(), DriverArgs));
   if (!RocmInstallation.checkCommonBitcodeLibs(CanonArch, LibDeviceFile,
                                                ABIVer))
     return {};
 
   // If --hip-device-lib is not set, add the default bitcode libraries.
   // TODO: There are way too many flags that change this. Do we need to check

clang/lib/Driver/ToolChains/HIPAMD.cpp

void HIPAMDToolChain::addClangTargetOptions(
   if (!DriverArgs.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc,
                           false))
     CC1Args.append({"-mllvm", "-amdgpu-internalize-symbols"});
 
   StringRef MaxThreadsPerBlock =
       DriverArgs.getLastArgValue(options::OPT_gpu_max_threads_per_block_EQ);
   if (!MaxThreadsPerBlock.empty()) {
     std::string ArgStr =
-        std::string("--gpu-max-threads-per-block=") + MaxThreadsPerBlock.str();
+        (Twine("--gpu-max-threads-per-block=") + MaxThreadsPerBlock).str();
     CC1Args.push_back(DriverArgs.MakeArgStringRef(ArgStr));
   }
 
   CC1Args.push_back("-fcuda-allow-variadic-functions");
 
   // Default to "hidden" visibility, as object level linking will not be
   // supported for the foreseeable future.
   if (!DriverArgs.hasArg(options::OPT_fvisibility_EQ,

 llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
 HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
   llvm::SmallVector<BitCodeLibraryInfo, 12> BCLibs;
   if (DriverArgs.hasArg(options::OPT_nogpulib))
     return {};
   ArgStringList LibraryPaths;
 
   // Find in --hip-device-lib-path and HIP_LIBRARY_PATH.
-  for (auto Path : RocmInstallation.getRocmDeviceLibPathArg())
+  for (StringRef Path : RocmInstallation.getRocmDeviceLibPathArg())
     LibraryPaths.push_back(DriverArgs.MakeArgString(Path));
 
   addDirectoryList(DriverArgs, LibraryPaths, "", "HIP_DEVICE_LIB_PATH");
 
   // Maintain compatability with --hip-device-lib.
   auto BCLibArgs = DriverArgs.getAllArgValues(options::OPT_hip_device_lib_EQ);
   if (!BCLibArgs.empty()) {
     llvm::for_each(BCLibArgs, [&](StringRef BCName) {
       StringRef FullName;
-      for (std::string LibraryPath : LibraryPaths) {
+      for (StringRef LibraryPath : LibraryPaths) {
         SmallString<128> Path(LibraryPath);
         llvm::sys::path::append(Path, BCName);
         FullName = Path;
         if (llvm::sys::fs::exists(FullName)) {
           BCLibs.push_back(FullName);
           return;
         }
       }
if (DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
         unsigned DiagID = getDriver().getDiags().getCustomDiagID(
             DiagnosticsEngine::Error,
             "AMDGPU address sanitizer runtime library (asanrtl) is not found. "
             "Please install ROCm device library which supports address "
             "sanitizer");
         getDriver().Diag(DiagID);
         return {};
       } else
-        BCLibs.push_back({AsanRTL.str(), /*ShouldInternalize=*/false});
+        BCLibs.emplace_back(AsanRTL, /*ShouldInternalize=*/false);
     }
 
     // Add the HIP specific bitcode library.
     BCLibs.push_back(RocmInstallation.getHIPPath());
 
     // Add common device libraries like ocml etc.
-    for (auto N : getCommonDeviceLibNames(DriverArgs, GpuArch.str()))
-      BCLibs.push_back(StringRef(N));
+    for (StringRef N : getCommonDeviceLibNames(DriverArgs, GpuArch.str()))
+      BCLibs.emplace_back(N);
 
     // Add instrument lib.
     auto InstLib =
         DriverArgs.getLastArgValue(options::OPT_gpu_instrument_lib_EQ);
     if (InstLib.empty())
       return BCLibs;
     if (llvm::sys::fs::exists(InstLib))
       BCLibs.push_back(InstLib);

clang/lib/Driver/ToolChains/ROCm.h

public:
   StringRef getABIVersionPath(DeviceLibABIVersion ABIVer) const {
     auto Loc = ABIVersionMap.find(ABIVer.ABIVersion);
     if (Loc == ABIVersionMap.end())
       return StringRef();
     return Loc->second;
   }
 
   /// Get libdevice file for given architecture
-  std::string getLibDeviceFile(StringRef Gpu) const {
-    return LibDeviceMap.lookup(Gpu);
+  StringRef getLibDeviceFile(StringRef Gpu) const {
+    auto Loc = LibDeviceMap.find(Gpu);
+    if (Loc == LibDeviceMap.end())
+      return "";
+    return Loc->second;
   }
 
   void AddHIPIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                          llvm::opt::ArgStringList &CC1Args) const;
 
   void detectDeviceLibrary();
   void detectHIPRuntime();
 
   /// Get the values for --rocm-device-lib-path arguments
-  std::vector<std::string> getRocmDeviceLibPathArg() const {
+  const ArrayRef<std::string> getRocmDeviceLibPathArg() const {
     return RocmDeviceLibPathArg;
   }
 
   /// Get the value for --rocm-path argument
   StringRef getRocmPathArg() const { return RocmPathArg; }
 
   /// Get the value for --hip-version argument
   StringRef getHIPVersionArg() const { return HIPVersionArg; }
 
-  std::string getHIPVersion() const { return DetectedVersion; }
+  StringRef getHIPVersion() const { return DetectedVersion; }
 };
 
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ROCM_H