[PATCH] D158250: [IR] Add more details to StructuralHash

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/IR/StructuralHash.cpp:37
+    uint64_t ToAdd = hash_combine(V);
+    Hash = hashing::detail::hash_16_bytes(Hash, ToAdd);
+  }
----------------
Call `hash(ToAdd)` instead?


================
Comment at: llvm/lib/IR/StructuralHash.cpp:51
+    else if (ValueType->isDoubleTy())
+      hash(1002);
+  }
----------------
Aren't these cases covered by the type ID hash?


================
Comment at: llvm/lib/IR/StructuralHash.cpp:86
+    // semantic differences in the IR.
+    if (const CmpInst *ComparisonInstruction = dyn_cast<CmpInst>(&Inst))
+      hash(ComparisonInstruction->getPredicate());
----------------
Use auto with dyn_cast.


================
Comment at: llvm/lib/IR/StructuralHash.cpp:87
+    if (const CmpInst *ComparisonInstruction = dyn_cast<CmpInst>(&Inst))
+      hash(ComparisonInstruction->getPredicate());
+
----------------
Hrm, apparently we don't have a public API to get subclass data in a generic way :/


================
Comment at: llvm/lib/IR/StructuralHash.cpp:90
+    for (unsigned I = 0; I < Inst.getNumOperands(); ++I)
+      updateOperand(Inst.getOperand(I));
+  }
----------------
Loop over `operands()`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158250/new/

https://reviews.llvm.org/D158250