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

[Johannes Doerfert via llvm-commits]

jdoerfert added a comment.

@Tobias @Michael:

  First, thanks for you your thoughts on this patch! Second, I agree on
  basically all the key facts, both positive and negative. Nevertheless,
  I like to go forward with the general idea as I think the positive
  aspects outweigh the negative ones.

@Tobias: I can come up with an improved patch that is less complex and

  places only needed PHI nodes. Would that be enough to get it in
  if we do not show regressions? If not I will not invest any
  more time.

> 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

> 

> - You received this message because you are subscribed to the Google Groups "Polly Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to polly-dev+unsubscribe at googlegroups.com. For more options, visit https://groups.google.com/d/optout.




- F1858238: signature.asc <http://reviews.llvm.org/F1858238>


http://reviews.llvm.org/D15722