Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

Heterogenous lookups allow us to call find with StringRef, avoiding a
temporary heap allocation of std::string.

Heterogenous lookups allow us to call find with StringRef, avoiding a
temporary heap allocation of std::string.
@kazutakahirata kazutakahirata requested a review from nikic November 7, 2024 04:21
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Nov 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

@llvm/pr-subscribers-clang-driver

Author: Kazu Hirata (kazutakahirata)

Changes

Heterogenous lookups allow us to call find with StringRef, avoiding a
temporary heap allocation of std::string.


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

1 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/HIPUtility.cpp (+5-6)
diff --git a/clang/lib/Driver/ToolChains/HIPUtility.cpp b/clang/lib/Driver/ToolChains/HIPUtility.cpp
index c8075cbfe36b35..3f81c3cb0f80e8 100644
--- a/clang/lib/Driver/ToolChains/HIPUtility.cpp
+++ b/clang/lib/Driver/ToolChains/HIPUtility.cpp
@@ -148,8 +148,8 @@ class HIPUndefinedFatBinSymbols {
   bool Verbose;
   std::set<std::string> FatBinSymbols;
   std::set<std::string> GPUBinHandleSymbols;
-  std::set<std::string> DefinedFatBinSymbols;
-  std::set<std::string> DefinedGPUBinHandleSymbols;
+  std::set<std::string, std::less<>> DefinedFatBinSymbols;
+  std::set<std::string, std::less<>> DefinedGPUBinHandleSymbols;
   const std::string FatBinPrefix = "__hip_fatbin";
   const std::string GPUBinHandlePrefix = "__hip_gpubin_handle";
 
@@ -260,11 +260,10 @@ class HIPUndefinedFatBinSymbols {
 
       // Add undefined symbols if they are not in the defined sets
       if (isFatBinSymbol &&
-          DefinedFatBinSymbols.find(Name.str()) == DefinedFatBinSymbols.end())
+          DefinedFatBinSymbols.find(Name) == DefinedFatBinSymbols.end())
         FatBinSymbols.insert(Name.str());
-      else if (isGPUBinHandleSymbol &&
-               DefinedGPUBinHandleSymbols.find(Name.str()) ==
-                   DefinedGPUBinHandleSymbols.end())
+      else if (isGPUBinHandleSymbol && DefinedGPUBinHandleSymbols.find(Name) ==
+                                           DefinedGPUBinHandleSymbols.end())
         GPUBinHandleSymbols.insert(Name.str());
     }
   }

@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

@llvm/pr-subscribers-clang

Author: Kazu Hirata (kazutakahirata)

Changes

Heterogenous lookups allow us to call find with StringRef, avoiding a
temporary heap allocation of std::string.


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

1 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/HIPUtility.cpp (+5-6)
diff --git a/clang/lib/Driver/ToolChains/HIPUtility.cpp b/clang/lib/Driver/ToolChains/HIPUtility.cpp
index c8075cbfe36b35..3f81c3cb0f80e8 100644
--- a/clang/lib/Driver/ToolChains/HIPUtility.cpp
+++ b/clang/lib/Driver/ToolChains/HIPUtility.cpp
@@ -148,8 +148,8 @@ class HIPUndefinedFatBinSymbols {
   bool Verbose;
   std::set<std::string> FatBinSymbols;
   std::set<std::string> GPUBinHandleSymbols;
-  std::set<std::string> DefinedFatBinSymbols;
-  std::set<std::string> DefinedGPUBinHandleSymbols;
+  std::set<std::string, std::less<>> DefinedFatBinSymbols;
+  std::set<std::string, std::less<>> DefinedGPUBinHandleSymbols;
   const std::string FatBinPrefix = "__hip_fatbin";
   const std::string GPUBinHandlePrefix = "__hip_gpubin_handle";
 
@@ -260,11 +260,10 @@ class HIPUndefinedFatBinSymbols {
 
       // Add undefined symbols if they are not in the defined sets
       if (isFatBinSymbol &&
-          DefinedFatBinSymbols.find(Name.str()) == DefinedFatBinSymbols.end())
+          DefinedFatBinSymbols.find(Name) == DefinedFatBinSymbols.end())
         FatBinSymbols.insert(Name.str());
-      else if (isGPUBinHandleSymbol &&
-               DefinedGPUBinHandleSymbols.find(Name.str()) ==
-                   DefinedGPUBinHandleSymbols.end())
+      else if (isGPUBinHandleSymbol && DefinedGPUBinHandleSymbols.find(Name) ==
+                                           DefinedGPUBinHandleSymbols.end())
         GPUBinHandleSymbols.insert(Name.str());
     }
   }

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LG

@kazutakahirata kazutakahirata merged commit 7bd9be2 into llvm:main Nov 7, 2024
11 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_001_heterogenous_Driver branch November 7, 2024 18:54
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
Heterogenous lookups allow us to call find with StringRef, avoiding a
temporary heap allocation of std::string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants