[AArch64][GISEL] Reduce likelihood of hash collisions for mappings in RegisterBankInfo

[marcauberer]

Fixes #85209

This patch removes the truncation from hash_code aka size_t down to unsigned, that currently happens on DenseMap accesses in RegisterBankInfo. This reduces the likelihood of hash collisions, as well as the likelihood of hitting EmptyKey or TombstoneKey, the special key values of DenseMap. This is not the ultimate solution to the problem, but we can do it in any case.

[llvmbot]

@llvm/pr-subscribers-llvm-regalloc

@llvm/pr-subscribers-llvm-adt

Author: Marc Auberer (marcauberer)

Changes

Fixes #85209

This patch removes the truncation from hash_code aka size_t down to unsigned, that currently happens on DenseMap accesses in RegisterBankInfo. This reduces the likelihood of hash collisions, as well as the likelihood of hitting EmptyKey or TombstoneKey, the special key values of DenseMap. This is not the ultimate solution to the problem, but we can do it in any case.

---

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

3 Files Affected:

• (modified) llvm/include/llvm/ADT/DenseMap.h (-1)
• (modified) llvm/include/llvm/CodeGen/RegisterBankInfo.h (+4-4)
• (modified) llvm/lib/CodeGen/RegisterBankInfo.cpp (+2-3)

diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index 3ef6a7cd1b4b58..51aa555d29f78c 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -28,7 +28,6 @@
 #include <cstring>
 #include <initializer_list>
 #include <iterator>
-#include <new>
 #include <type_traits>
 #include <utility>
 
diff --git a/llvm/include/llvm/CodeGen/RegisterBankInfo.h b/llvm/include/llvm/CodeGen/RegisterBankInfo.h
index 62c4a57a605d68..9704e3b1fdedd6 100644
--- a/llvm/include/llvm/CodeGen/RegisterBankInfo.h
+++ b/llvm/include/llvm/CodeGen/RegisterBankInfo.h
@@ -399,22 +399,22 @@ class RegisterBankInfo {
 
   /// Keep dynamically allocated PartialMapping in a separate map.
   /// This shouldn't be needed when everything gets TableGen'ed.
-  mutable DenseMap<unsigned, std::unique_ptr<const PartialMapping>>
+  mutable DenseMap<hash_code, std::unique_ptr<const PartialMapping>>
       MapOfPartialMappings;
 
   /// Keep dynamically allocated ValueMapping in a separate map.
   /// This shouldn't be needed when everything gets TableGen'ed.
-  mutable DenseMap<unsigned, std::unique_ptr<const ValueMapping>>
+  mutable DenseMap<hash_code, std::unique_ptr<const ValueMapping>>
       MapOfValueMappings;
 
   /// Keep dynamically allocated array of ValueMapping in a separate map.
   /// This shouldn't be needed when everything gets TableGen'ed.
-  mutable DenseMap<unsigned, std::unique_ptr<ValueMapping[]>>
+  mutable DenseMap<hash_code, std::unique_ptr<ValueMapping[]>>
       MapOfOperandsMappings;
 
   /// Keep dynamically allocated InstructionMapping in a separate map.
   /// This shouldn't be needed when everything gets TableGen'ed.
-  mutable DenseMap<unsigned, std::unique_ptr<const InstructionMapping>>
+  mutable DenseMap<hash_code, std::unique_ptr<const InstructionMapping>>
       MapOfInstructionMappings;
 
   /// Getting the minimal register class of a physreg is expensive.
diff --git a/llvm/lib/CodeGen/RegisterBankInfo.cpp b/llvm/lib/CodeGen/RegisterBankInfo.cpp
index 72b07eb1902d9b..551af75dff0bd3 100644
--- a/llvm/lib/CodeGen/RegisterBankInfo.cpp
+++ b/llvm/lib/CodeGen/RegisterBankInfo.cpp
@@ -221,8 +221,8 @@ RegisterBankInfo::getInstrMappingImpl(const MachineInstr &MI) const {
       if (!OperandsMapping[0]) {
         if (MI.isRegSequence()) {
           // For reg_sequence, the result size does not match the input.
-          unsigned ResultSize = getSizeInBits(MI.getOperand(0).getReg(),
-                                              MRI, TRI);
+          unsigned ResultSize =
+              getSizeInBits(MI.getOperand(0).getReg(), MRI, TRI);
           OperandsMapping[0] = &getValueMapping(0, ResultSize, *CurRegBank);
         } else {
           OperandsMapping[0] = ValMapping;
@@ -332,7 +332,6 @@ RegisterBankInfo::getValueMapping(const PartialMapping *BreakDown,
 template <typename Iterator>
 const RegisterBankInfo::ValueMapping *
 RegisterBankInfo::getOperandsMapping(Iterator Begin, Iterator End) const {
-
   ++NumOperandsMappingsAccessed;
 
   // The addresses of the value mapping are unique.

[arsenm]

LGTM, except for the unrelated pieces in the diff here