[PATCH] D22143: [SimplifyCFG] Rewrite SinkThenElseCodeToEnd

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 9 11:20:18 PDT 2016


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

lgtm with minor comments inline.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:1328
@@ +1327,3 @@
+// depends on control flow, the \c At0 and \c At1 parameters specify a
+// location for the query. This function is essentially answering the/
+// query "If V0 were moved to At0, and V1 were moved to At1, are V0 and V1
----------------
Stray `/` at EOL?

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:1369
@@ +1368,3 @@
+  // but we currently don't have that available.
+  if (std::any_of(std::next(I0->getIterator()), At0, [](const Instruction &I) {
+        return I.mayReadOrWriteMemory();
----------------
Optional, but I'd extract out a `WritesMemory` lambda out and use it twice.  I suspect the conditions will be more readable that way.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:1442
@@ +1441,3 @@
+      // Don't touch any operand of token type.
+      return false;
+    if (!all_of(Insts, [&I0, OI](const Instruction *I) {
----------------
Again, I personally would've extracted out a named `SameAsI0` lambda here.  That way the condition will read more naturally.


Repository:
  rL LLVM

https://reviews.llvm.org/D22143





More information about the llvm-commits mailing list