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

Fri Mar 8 01:33:45 PST 2019

dmgreen added a comment.

Hello. I think the idea of this sounds sensible, to limit things if the instruction are too far apart, I'm just not sure of using domtree's to do that.

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1196
+    unsigned CmpF = CmpNode->getDFSNumOut();
+    unsigned Distance = MathDominates ?
+      std::min(CmpD - MathD, MathF - CmpF) :
----------------
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.

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1199
+      std::min(MathD - CmpD, CmpF - MathF);
+    if (Distance > 2)
+      return false;
----------------
2 is a magic number? Is it worth making a cl:opt for it, so it at least has a name?

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

https://reviews.llvm.org/D59024