[clang] a446827 - [NFC][Clang][Driver][AMDGPU] Avoid temporary copies of std::string by using Twine and StringRef
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 5 16:13:45 PST 2022
On Mon, Dec 5, 2022 at 5:27 AM Juan Manuel MARTINEZ CAAMAÑO via
cfe-commits <cfe-commits at lists.llvm.org> wrote:
>
>
> Author: Juan Manuel MARTINEZ CAAMAÑO
> Date: 2022-12-05T07:27:10-06:00
> New Revision: a446827249bdeb2f27e55a9f4942bd7425ecb0ff
>
> URL: https://github.com/llvm/llvm-project/commit/a446827249bdeb2f27e55a9f4942bd7425ecb0ff
> DIFF: https://github.com/llvm/llvm-project/commit/a446827249bdeb2f27e55a9f4942bd7425ecb0ff.diff
>
> LOG: [NFC][Clang][Driver][AMDGPU] Avoid temporary copies of std::string by using Twine and StringRef
>
> Reviewed By: tra
>
> Differential Revision: https://reviews.llvm.org/D139023
>
> Added:
>
>
> Modified:
> clang/lib/Driver/ToolChains/AMDGPU.cpp
> clang/lib/Driver/ToolChains/HIPAMD.cpp
> clang/lib/Driver/ToolChains/ROCm.h
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp
> index f3ac7d2ab735..ff77969bbec5 100644
> --- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
> +++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
> @@ -854,7 +854,7 @@ void ROCMToolChain::addClangTargetOptions(
> 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,
> @@ -946,7 +946,7 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &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,
>
> diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp
> index c64421d259ce..a555fe5830e0 100644
> --- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
> +++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
> @@ -237,7 +237,7 @@ void HIPAMDToolChain::addClangTargetOptions(
> 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));
> }
>
> @@ -344,7 +344,7 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
> 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");
> @@ -354,7 +354,7 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
> 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;
> @@ -387,15 +387,15 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
> 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 =
>
> diff --git a/clang/lib/Driver/ToolChains/ROCm.h b/clang/lib/Driver/ToolChains/ROCm.h
> index b16deecdebec..600c8b39f4b3 100644
> --- a/clang/lib/Driver/ToolChains/ROCm.h
> +++ b/clang/lib/Driver/ToolChains/ROCm.h
> @@ -252,8 +252,11 @@ class RocmInstallationDetector {
> }
>
> /// 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,
> @@ -263,7 +266,7 @@ class RocmInstallationDetector {
> void detectHIPRuntime();
>
> /// Get the values for --rocm-device-lib-path arguments
> - std::vector<std::string> getRocmDeviceLibPathArg() const {
> + const ArrayRef<std::string> getRocmDeviceLibPathArg() const {
Generally return values shouldn't be `const` - since there's a copy
being made here anyway. Can you remove the `const`?
> return RocmDeviceLibPathArg;
> }
>
> @@ -273,7 +276,7 @@ class RocmInstallationDetector {
> /// 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
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list