[PATCH] D37467: Add a new pass to speculate around PHI nodes with constant (integer) operands when profitable.

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

In https://reviews.llvm.org/D37467#889452, @chandlerc wrote:

> I've taken some time to go through the code in NewGVN that computes similar things based on the suggestion from Danny. These now do *very* similar things in terms of walk. The differences I've seen are:
>
> 1. While the safety checks are both preorder, my code uses DFS instead of BFS. The reason is that we walk up the stack to mark more nodes as unsafe when we discover an unsafe node.


I originally wrote mine as a DFS, i may go back to it.
I actually originally had graphtraits set up so it could be used with a depth_first_iterator on operands and walk the def-use chain.  I wonder if i should do that and we could share *that*, or whether it's not worth it (obviously, not something we have to do this second, and i could look at it as a followup).

> 2. The safety checks in this pass are stricter than in NewGVN because other parts of NewGVN handle various safety concerns of code motion. We essentially merge them here and use stricter checks
> 3. The cost modeling does DFS postorder. I don't see a way around this given what the cost model is trying to do. It lets us use dynamic programming to avoid recomputing the same cost multiple times. Fortunately, this walk never fails and we use the above preorder-checked safety walk to establish the total set of nodes traversed.
> 
>   I suspect this will make it hard and unlikely to be useful to share a lot of code between the two. =/

:(
 They are solving similar but ultimately different problems and have different tradeoffs as a consequence.

> Anyways, I think the patch as is is probably good for review now.

I'm on vacation, but i'll try to look at it late next week if nobody beats me to it.


https://reviews.llvm.org/D37467