[PATCH] D38594: [InlineCost] Tracking Values through PHI Nodes

[Easwaran Raman via Phabricator via llvm-commits]

eraman added a comment.

Thanks for working on this!



================
Comment at: lib/Analysis/InlineCost.cpp:167
 
+  /// Kepp track of dead blocks due to the constant arguments.
+  SetVector<BasicBlock *> DeadBlocks;
----------------
nit: Kepp -> Keep


================
Comment at: lib/Analysis/InlineCost.cpp:170
+
+  /// The mapping of the blocks to their known successors due to the constant
+  /// arguments.
----------------
known successors -> known unique successors may be helpful here . 


================
Comment at: lib/Analysis/InlineCost.cpp:495
+
+      continue;
+    }
----------------
If you have a phi with null as one incoming value and a non-null pointer with a constant offset as the other incoming argument, this code would incorrectly treat the result of the phi as null. Inside the loop, both FirstC and FirstV will be non-null and after exiting the loop , SimplifiedValues[&I] will be set to FirstC. 


================
Comment at: lib/Analysis/InlineCost.cpp:1626
+        // Continue growing the dead block lists.  If all the predessors of the
+        // dead block's successor are all dead, the successor is also dead.
+        for (BasicBlock *S : successors(Dead)) {
----------------
This doesn't consider the case where a dead block's successor S has a  predecessor P that is not dead, but that predecessor P has a known successor that is not S. Why not extend 1630 below to

return DeadBlocks.count(P) || (KnownSuccessors[P]  && KnownSuccessors[P] != S)



Repository:
  rL LLVM

https://reviews.llvm.org/D38594