[PATCH] D59024: [CGP] Limit distance between overflow math and cmp

[Sam Parker via Phabricator via llvm-commits]

samparker marked an inline comment as done.
samparker added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1196
+    unsigned CmpF = CmpNode->getDFSNumOut();
+    unsigned Distance = MathDominates ?
+      std::min(CmpD - MathD, MathF - CmpF) :
----------------
dmgreen wrote:
> I'm not sure this will reliably calculate a meaningful distance for a lot of trees. And I'm not sure that the children of a node are necessarily always in the same deterministic order. The node level may be better, but the distance in the domtree is probably not be the best measure.
> 
> Maybe, because the Distance is small, can we just search back through preds to find the block? We already know which one dominates, and most times they will be in the same block.  We would have to be careful of compile time, but if we don't have to search more that Distance, and Distance is fairly small, hopefully it will work OK.
This sounds perfectly reasonable to me. I'll do a BFS and limit the depth using the cl::opt.


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

https://reviews.llvm.org/D59024