[PATCH] Move StructurizeCFG out of R600 to generic Transforms

[Owen Anderson]

Hey,

I have a serious question about this patch that I raised in a prior email, and I'd appreciate it if you'd address it rather than continuing in the opposite direction.

On Jun 5, 2013, at 12:42 PM, Matt Arsenault <Matthew.Arsenault at amd.com> wrote:

>  Move NearestCommonDominator to separate file
> 
> http://llvm-reviews.chandlerc.com/D906
> 
> CHANGE SINCE LAST DIFF
>  http://llvm-reviews.chandlerc.com/D906?vs=2252&id=2279#toc
> 
> Files:
>  include/llvm/Analysis/NearestCommonDominator.h

I don't understand why this analysis is necessary.  We already have pair-wise nearestCommonDominator functionality built into DomTree.  If it needs to be extended to support multi-block NCD queries, that should be possible to implement on top of the existing functionality rather than build a new analysis on top of it.

>  include/llvm/InitializePasses.h
>  include/llvm/LinkAllPasses.h
>  include/llvm/Transforms/Scalar.h
>  lib/Target/R600/AMDGPUStructurizeCFG.cpp
>  lib/Target/R600/AMDGPUTargetMachine.cpp
>  lib/Target/R600/CMakeLists.txt
>  lib/Transforms/Scalar/CMakeLists.txt
>  lib/Transforms/Scalar/Scalar.cpp
>  lib/Transforms/Scalar/StructurizeCFG.cpp

More generally,  I don't understand why it's desirable to move this out of the R600 target.  The comments hand wave about it being useful for other targets, but I don't see anyone actually planning to use it.  If there is interest in it from other targets, then you probably need to start by having a conversation with other target authors about what kind of common canonical form different targets can share rather than simply foisting whatever was convenient for R600 onto them.

--Owen
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130605/7b47bdd6/attachment.html>