[PATCH] D30116: Refactor SimplifyCFG:canSinkInstructions [NFC]

[James Molloy via Phabricator via llvm-commits]

jmolloy requested changes to this revision.
jmolloy added a comment.
This revision now requires changes to proceed.

Hi,

Can you please explain what you are doing and why? Is there a functional change here? or just moving code around?

I can see a functional change



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1454
   const Instruction *I0 = Insts.front();
-  for (auto *I : Insts)
+  for (Instruction *I : Insts)
     if (!I->isSameOperationAs(I0))
----------------
Why?


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1465
     auto *Succ = I0->getParent()->getTerminator()->getSuccessor(0);
-    if (!all_of(Insts, [&PNUse,&Succ](const Instruction *I) -> bool {
+    if (!PNUse || PNUse->getParent() != Succ)
+      return false;
----------------
This change looks fine.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1488
-    // https://llvm.org/bugs/show_bug.cgi?id=30188
-    if (OI == 1 && isa<StoreInst>(I0) &&
-        any_of(Insts, [](const Instruction *I) {
----------------
Your new code is ignoring the operand index, so is more conservative than the old code.


https://reviews.llvm.org/D30116