[PATCH] D73152: [PHIElimination] Compile time optimization for huge functions.

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/CodeGen/PHIElimination.cpp:169
+        unsigned VirtReg = Register::index2VirtReg(Index);
+        MachineInstr *DefMI = MRI->getVRegDef(VirtReg);
+        if (!DefMI)
----------------
I think it is a little bit ugly to do this after setting leaveSSA above.
My suggestion is to postpone the leaveSSA toggling until after populating the LiveInSets (considering that this code relies on still having ssa form).


================
Comment at: llvm/lib/CodeGen/PHIElimination.cpp:180
+        MachineBasicBlock *DefMBB = DefMI->getParent();
+        if (VI.Kills.size() > 1 || VI.Kills.front()->getParent() != DefMBB)
+          for (auto *MI : VI.Kills)
----------------
This might needs an explanatory code comment.

If there is more than one kill, then it isn't killed in DefMBB => add to liveinset. (Ok!)

Else, if there is one kill and it isn't DefMBB  => add to liveinset. (Ok!)

What I think is missing here is a guard to avoid the Kills.front() deref when there are no kills (may happen when a phi is the last use).




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

https://reviews.llvm.org/D73152