[PATCH] D15722: [WIP][Polly] SSA Codegen

[Tobias Grosser via llvm-commits]

grosser added a comment.

Hi Johannes,

I did not get along to go through this in detail, but wanted to provide some first feedback.

First, thanks a lot for looking into ways to make Polly generate good scalar code without further preprocessing!

Here some first comments on the general approach:

1. (you already know about this) I am concerned that this approach might be more complex and/or may have more corner cases we need to worry about. Aditya mentioned that in their implementation in graphite, they fail to generate code in various situations. My understanding is that you are not aware of any such limitations in your code and you already checked several of the examples Aditya provided. It would probably be good for Aditya and Sebastian to have a look at this patch, as they have experience with SSA code generation and might be able to

provide you with some test cases.

2. It is interesting to see that this patch actually removes a lot of code. So maybe I am wrong about point 1)

3. An alternative approach might be to just call PromoteMemToRegisters on the scalar allocs we introduced. This would allow us to keep the same mental model, but to still generate good code.

Some questions I have before i get into reviewing this code:

a) Does your patch need to make any effort to eliminate dead PHI nodes? Will we have a lot of PHI nodes that might need to be dead-code eliminated after your patch? More precisely, would we now need to run a phase of DCE after Polly instead of Mem2Reg.

b) Are there any remaining issues or inherent limitations you are aware in comparison to the old approach?

c) I know you are still running performance numbers, which will be interesting to look at. Otherwise, is this patch ready to be reviewed.


http://reviews.llvm.org/D15722