[PATCH] D120177: [BOLT] CMOVConversion pass
Amir Ayupov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 4 16:16:01 PST 2022
Amir marked an inline comment as done.
Amir added inline comments.
================
Comment at: bolt/lib/Passes/CMOVConversion.cpp:66
+ return false;
+ return true;
+}
----------------
rafauler wrote:
> Isn't it necessary to also check that LHS has a common predecessor with RHS? Edit: I saw how the main function uses this and this assumption is implied. Perhaps add a comment "caller guarantees that LHS and RHS share the same predecessor".
Added both
================
Comment at: bolt/lib/Passes/CMOVConversion.cpp:155
+ // successor.
+ auto RPO = ReversePostOrderTraversal<BinaryFunction *>(&Function);
+ bool Modified = false;
----------------
rafauler wrote:
> Is this necessary? Can't we just traverse in BF->BB vector order?
Yes, post-order traversal is necessary to guarantee that we can collapse the hammock into a straight-line code (merge Predecessor, RHS and LHS into a single BB). Actually, I've made a mistake here as we need to traverse bottom-up (post-order, not RPO). Added a back-to-back hammocks test for that
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120177/new/
https://reviews.llvm.org/D120177
More information about the llvm-commits
mailing list