[PATCH] D39352: [SimplifyCFG] Don't do if-conversion if there is a long dependence chain

[Hal Finkel via Phabricator via llvm-commits]

hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

LGTM, however, the testing here is fairly light. We should have some cases where we do perform the if conversion because of the DependenceChainLatency check, where we do perform the if conversion because of the IPC check, and because of the BB size check. Also, where the if-converted block has some non-trivial adjustment because of LatencyAdjustment.



================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:419
+/// LongestChain is true, or the dependence chain containing the compare
+/// instruction.
+static unsigned FindDependenceChainLatency(BasicBlock *BB,
----------------
the compare instruction. -> the compare instruction feeding the block's conditional branch.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:506
+/// if the branch has a high miss rate, because the data dependence is changed
+/// into control dependence, and control dependence can be speculated, and thus,
+/// the second part can execute in parallel with the first part on modern OOO
----------------
... because the data dependence is changed into control dependence, and ...

This is backward. It should say, "... because the control dependence is transformed into a data dependence, and the control dependence can be speculated, and ...".



================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:532
+  // We check small BB only since it is more difficult to find unrelated
+  // instructions to fill functional units in small BB.
+  if (NewBBSize > SmallBBSize)
----------------
in small BB. -> in a small BB.


https://reviews.llvm.org/D39352