[PATCH] D54007: Use a data structure better suited for large sets in SimplificationTracker.

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

If I remember correctly a problem was that depending on the order iteration at line 3011

  while (PhiNodesToMatch.size())

the end result (which PHI node that others are found out to be equivalent to) could be different. Quite annoying when trying to debug something, and when turning on -print-after-all or rebuilding using -g the result is different.

Have you, for example, tested to run llc with and without -print-after-all, or -debug, or some other options that might impact memory layout. And have you checked that we still are debug invariant (always getting the same result regardless if you build the compiler using -g, or a different -O level)?
Another way to test it could be to build two versions of llc. One that inserts at the end of AllPhiNodes and one the inserts at the front (using the SmallSetVector and some hacks to insert from different ends in the vector). Then run a bunch of tests and compare the result.

Btw, when you say 600.000 PHI nodes, is that in the same SimplificationTracker instance?
If 600.000 is some kind of total, what is the average size of AllPhiNodes when MatchPhiSet is called? Perhaps the size N (currently set to 32) of the SmallSetVector can be tuned.



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2651
   const SimplifyQuery &SQ;
   // Tracks newly created Phi nodes. We use a SetVector to get deterministic
   // order when iterating over the set in MatchPhiSet.
----------------
The comment about SetVector is no longer valid with this patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D54007