Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

This patch simplifies the AddInteger overloads by introducing
AddIntegerImpl, a helper function to handle all cases, both 32-bit and
64-bit cases.

This patch simplifies the AddInteger overloads by introducing
AddIntegerImpl, a helper function to handle all cases, both 32-bit and
64-bit cases.
@llvmbot
Copy link
Member

llvmbot commented Oct 23, 2025

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

This patch simplifies the AddInteger overloads by introducing
AddIntegerImpl, a helper function to handle all cases, both 32-bit and
64-bit cases.


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

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/FoldingSet.h (+14-18)
diff --git a/llvm/include/llvm/ADT/FoldingSet.h b/llvm/include/llvm/ADT/FoldingSet.h
index 82a88c440ff24..675b5c6a35bbc 100644
--- a/llvm/include/llvm/ADT/FoldingSet.h
+++ b/llvm/include/llvm/ADT/FoldingSet.h
@@ -332,6 +332,14 @@ class FoldingSetNodeID {
   /// Use a SmallVector to avoid a heap allocation in the common case.
   SmallVector<unsigned, 32> Bits;
 
+  template <typename T> void AddIntegerImpl(T I) {
+    static_assert(std::is_integral_v<T> && sizeof(T) <= sizeof(unsigned) * 2,
+                  "T must be an integer type no wider than 64 bits");
+    Bits.push_back(static_cast<unsigned>(I));
+    if constexpr (sizeof(unsigned) < sizeof(T))
+      Bits.push_back(static_cast<unsigned long long>(I) >> 32);
+  }
+
 public:
   FoldingSetNodeID() = default;
 
@@ -348,24 +356,12 @@ class FoldingSetNodeID {
                   "unexpected pointer size");
     AddInteger(reinterpret_cast<uintptr_t>(Ptr));
   }
-  void AddInteger(signed I) { Bits.push_back(I); }
-  void AddInteger(unsigned I) { Bits.push_back(I); }
-  void AddInteger(long I) { AddInteger((unsigned long)I); }
-  void AddInteger(unsigned long I) {
-    if (sizeof(long) == sizeof(int))
-      AddInteger(unsigned(I));
-    else if (sizeof(long) == sizeof(long long)) {
-      AddInteger((unsigned long long)I);
-    } else {
-      llvm_unreachable("unexpected sizeof(long)");
-    }
-  }
-  void AddInteger(long long I) { AddInteger((unsigned long long)I); }
-  void AddInteger(unsigned long long I) {
-    AddInteger(unsigned(I));
-    AddInteger(unsigned(I >> 32));
-  }
-
+  void AddInteger(signed I) { AddIntegerImpl(I); }
+  void AddInteger(unsigned I) { AddIntegerImpl(I); }
+  void AddInteger(long I) { AddIntegerImpl(I); }
+  void AddInteger(unsigned long I) { AddIntegerImpl(I); }
+  void AddInteger(long long I) { AddIntegerImpl(I); }
+  void AddInteger(unsigned long long I) { AddIntegerImpl(I); }
   void AddBoolean(bool B) { AddInteger(B ? 1U : 0U); }
   LLVM_ABI void AddString(StringRef String);
   LLVM_ABI void AddNodeID(const FoldingSetNodeID &ID);

@kazutakahirata kazutakahirata merged commit 7dd06b3 into llvm:main Oct 23, 2025
12 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20251022_ADT_FoldingSet_if_constexpr branch October 23, 2025 06:42
mikolaj-pirog pushed a commit to mikolaj-pirog/llvm-project that referenced this pull request Oct 23, 2025
…64753)

This patch simplifies the AddInteger overloads by introducing
AddIntegerImpl, a helper function to handle all cases, both 32-bit and
64-bit cases.
dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
…64753)

This patch simplifies the AddInteger overloads by introducing
AddIntegerImpl, a helper function to handle all cases, both 32-bit and
64-bit cases.
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
…64753)

This patch simplifies the AddInteger overloads by introducing
AddIntegerImpl, a helper function to handle all cases, both 32-bit and
64-bit cases.
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
…64753)

This patch simplifies the AddInteger overloads by introducing
AddIntegerImpl, a helper function to handle all cases, both 32-bit and
64-bit cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants