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

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added inline comments.


================
Comment at: llvm/lib/CodeGen/PHIElimination.cpp:169
+        unsigned VirtReg = Register::index2VirtReg(Index);
+        MachineInstr *DefMI = MRI->getVRegDef(VirtReg);
+        if (!DefMI)
----------------
bjope wrote:
> 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).
OK - moved it down. 


================
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)
----------------
bjope wrote:
> 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).
> 
> 
Added comment.

> 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).

Ah, right!



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

https://reviews.llvm.org/D73152