[llvm] [StructuralHash] Refactor (PR #112621)

[Ellis Hoag via llvm-commits]

================
@@ -24,61 +24,86 @@ namespace {
 // by the MergeFunctions pass.
 
 class StructuralHashImpl {
-  uint64_t Hash = 4;
+  stable_hash Hash = 4;
 
-  void hash(uint64_t V) { Hash = hashing::detail::hash_16_bytes(Hash, V); }
+  bool DetailedHash;
 
   // This will produce different values on 32-bit and 64-bit systens as
   // hash_combine returns a size_t. However, this is only used for
   // detailed hashing which, in-tree, only needs to distinguish between
   // differences in functions.
-  template <typename T> void hashArbitaryType(const T &V) {
-    hash(hash_combine(V));
+  // TODO: This is not stable.
+  template <typename T> stable_hash hashArbitaryType(const T &V) {
+    return hash_combine(V);
   }
 
-  void hashType(Type *ValueType) {
-    hash(ValueType->getTypeID());
+  stable_hash hashType(Type *ValueType) {
+    SmallVector<stable_hash> Hashes;
+    Hashes.emplace_back(ValueType->getTypeID());
     if (ValueType->isIntegerTy())
-      hash(ValueType->getIntegerBitWidth());
+      Hashes.emplace_back(ValueType->getIntegerBitWidth());
+    return stable_hash_combine(Hashes);
   }
 
 public:
-  StructuralHashImpl() = default;
-
-  void updateOperand(Value *Operand) {
-    hashType(Operand->getType());
-
-    // The cases enumerated below are not exhaustive and are only aimed to
-    // get decent coverage over the function.
-    if (ConstantInt *ConstInt = dyn_cast<ConstantInt>(Operand)) {
-      hashArbitaryType(ConstInt->getValue());
-    } else if (ConstantFP *ConstFP = dyn_cast<ConstantFP>(Operand)) {
-      hashArbitaryType(ConstFP->getValue());
-    } else if (Argument *Arg = dyn_cast<Argument>(Operand)) {
-      hash(Arg->getArgNo());
-    } else if (Function *Func = dyn_cast<Function>(Operand)) {
+  StructuralHashImpl() = delete;
+  explicit StructuralHashImpl(bool DetailedHash) : DetailedHash(DetailedHash) {}
+
+  stable_hash hashConstant(Constant *C) {
+    SmallVector<stable_hash> Hashes;
+    // TODO: hashArbitaryType() is not stable.
+    if (ConstantInt *ConstInt = dyn_cast<ConstantInt>(C)) {
+      Hashes.emplace_back(hashArbitaryType(ConstInt->getValue()));
+    } else if (ConstantFP *ConstFP = dyn_cast<ConstantFP>(C)) {
+      Hashes.emplace_back(hashArbitaryType(ConstFP->getValue()));
+    } else if (Function *Func = dyn_cast<Function>(C))
       // Hashing the name will be deterministic as LLVM's hashing infrastructure
       // has explicit support for hashing strings and will not simply hash
       // the pointer.
-      hashArbitaryType(Func->getName());
-    }
+      Hashes.emplace_back(hashArbitaryType(Func->getName()));
----------------
ellishg wrote:

Let's keep the brackets here since the comment is long. In fact, I think the recommendation is to use brackets for all else cases now.

https://discourse.llvm.org/t/brace-rules-for-single-line-else-multi-line-if/82452?u=ellishg

https://github.com/llvm/llvm-project/pull/112621