[llvm-commits] [Review request] New StrongPHIElimination implementation

[Andy Trick]

Hi Cameron,

It's good to see a clean design for the coalescer. I only have one comment that I think is worth making before you finish the prototype. It looks like you may be introducing interferences when you generate copies. If so, the issue could show up deep within regalloc, or maybe even incorrect code. Rather than waste time debugging, can you implement a simple CSSA verifier? How about repeating the domtree walk after CSSA, time visiting the new copies, to verify that no interferences remain? The verification would not only help to you, but would be valuable to anyone who may touch coalescing in the future--especially if they don't understand it as well as you do.

-Andy

On Oct 5, 2010, at 3:23 PM, Cameron Zwarich wrote:

> I've been wanting to implement SSA-based register allocation in LLVM, but since that is a bit of a major task, I wanted to do something smaller to get myself more familiar with the backend. I decided to rewrite the StrongPHIElimination pass and get it closer to working. I split the pass up into 3 stages:
> 
> 1) AggressivePHICoalescing - this computes an aggressive coalescing of virtual registers that appear as operands or destinations of phis. It doesn't actually perform any coalescing, but it provides a data structure that later passes can query. This makes the code more modular, with no real change in performance, and it also means that the coalescing can be used for other purposes like SSA register allocation.
> 
> The implementation here is much slicker than the original, because it does away with the dominance forests entirely and performs all of the interference tests in a single depth-irst walk of the dominator tree. Phis are also modeled a bit differently, which makes it possible for StrongPHIElimination to work with critical edges.
> 
> 2) CSSA - this pass converts the program into Conventional SSA form by inserting copies based on the results of AggressivePHICoalescing. Conventional SSA is SSA with the restriction that phi-congruent variables do not have overlapping live ranges.
> 
> 3) StrongPHIElimination - since the program is now in CSSA form, StrongPHIElimination just renames phi-congruent registers, merges their live ranges, and deletes the phis.
> 
> Some comments on my patch:
> 
> 1) While the pass at least completes on the simple examples I tried (unlike the existing code), it causes SimpleRegisterCoalescing to hit an assertion failure. I am not certain whether this is due to a bug in the way the new code updates LiveIntervals or a bad assumption in SimpleRegisterCoalescing that is satisfied by PHIElimination but not StrongPHIElimination.
> 
> 2) I had to copy two functions from PHIElimination, SkipPHIsAndLabels() and FindCopyInsertPoint(). Presumedly, these should go somewhere shared, but where?
> 
> 3) It handles critical edges, but it still might be better to split them sometimes.
> 
> I've attached both a diff and a copy of the StrongPHIElimination.cpp file (as it got mangled pretty bad by the diff).
> 
> Cameron
> 
> <strong-phi.patch><StrongPHIElimination.cpp>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits