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

[Michael Kruse via llvm-commits]

Meinersbur added a comment.

For me the main argument for this patch is that it catches potential problems earlier (Your argument 1).

My secondary argument is that it reduces the overall complexity of the system. If currently and with http://reviews.llvm.org/F1855864 we'd do:
MemoryAccesses -> Load/Stores -> Registers
this patch would just do
MemoryAccesses -> Registers
That is, there is one step less, less concern about unpredictable interactions, and maybe even faster compilation.

I don't think there is a meaningful difference between http://reviews.llvm.org/F1855864 and running a mem2reg/SROA pass afterwards, as we do now, eg. in CodegenCleanup.

For me the size of a patch is no argument unless we are maintaining a legacy code base. While new code may introduce new issues that are to be fixed, this will be eventually compensated in the long term, even if the advantages are small. Number of lines is also not a useful metric for complexity/understandability.

My arguments against this patch would be:

- I actually find it harder to understand than the current. MemoryAccesses are modeled as READ/WRITE actions, so it is more straightforward to also just generate LoadInst/StoreInst.
- I am less than 100% sure that cases where it generates superlinear more PHIs than necessary never happen in practical code.

Hence, this comes down to what our priorities are. I'd vote in favor of SSA codegen.


http://reviews.llvm.org/D15722