[PATCH] D71396: [llvm][NFCi][lMIRVRegNamerUtils] Leverage MachineInstrExpressionTrait for hashing a MachineInstr.

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi added inline comments.


================
Comment at: llvm/lib/CodeGen/MIRVRegNamerUtils.cpp:62
+  SmallVector<unsigned, 16> MIOperands = {MI.getFlags()};
+  MIOperands.push_back(MachineInstrExpressionTrait::getHashValue(&MI));
 
----------------
aditya_nandakumar wrote:
> While I think this is cleaner, and I was aware of this, and one of the reasons I didn't use it initially is that for things such as PHI instructions, it just hashes the BB with just the pointer value (casted as integer) which is not guaranteed to be stable across different runs (but perfectly usable within the same run) which will affect the diffs.
> Perhaps a compromise which we can do is use the hash defined in the machine operand but opt out of hashing if we're dealing with MBB type.
Yeah agreed. I can revert back to https://reviews.llvm.org/D71396?id=233516 and try using hash_value directly on everything except for the specific things you've mentioned. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71396