[PATCH] D66029: llvm-canon

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi added inline comments.


================
Comment at: llvm/test/Transforms/IRCanonicalizer/reordering-instructions.ll:5
+entry:
+; CHECK: %a
+; CHECK: %c
----------------
consider making the last 3 check lines "CHECK-NEXT" 


================
Comment at: tools/llvm-canon/canonicalizer.cpp:88
+        // Initialize to a random constant, so the state isn't zero.
+        uint64_t Hash = 0x6acaa36bef8325c5ULL;
+
----------------
mpaszkowski wrote:
> fdeazeve wrote:
> > plotfi wrote:
> > > What is this magic number? Could this be generated with something like srand at the begining of runOnFunction? 
> > I might be missing something, but don't we want this algorithm to produce the same result if it's run on the "same" (two identical functions, modulo renaming, etc) function twice in a row?
> You are right, this number cannot be generated by srand. We want the canonicalization to be deterministic.
Ah yes, that makes a lot of sense. The MIR Canonicalizer ran into the same sort of issue. That's why the value-numbering-esque rewrite of it doesn't hash certain types of MachineOperands that might be different run to run. 


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

https://reviews.llvm.org/D66029