[PATCH] Move StructurizeCFG out of R600 to generic Transforms

Tom Stellard tom at stellard.net
Wed Jun 5 13:27:41 PDT 2013


On Wed, Jun 05, 2013 at 12:55:50PM -0700, Owen Anderson wrote:
> 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.
>

Hi Owen,

I'm not sure if there are any other in tree targets that could benefit from
the structurizer, but it will be useful to the AMDIL backend plus any
other out of tree GPU targets (Are there others?).  More importantly
though, part of Fransisco Jerez's GSoC project (TGSI backend) involves
making improvements to the structurizer and using it with the TGSI
backend, so I think moving it into common code is a good first step.

I didn't write the code, but as far as I can tell there is not really
anything specific to R600 in the structurizer.  In fact, there is a
R600 specific pass, which will stay in the R600 backend,
that is run after the structurizer to parse the CFG
and insert R600 target intrinsics to help the code generator.  So
conceivably all another target would have to do to use it would be to
write a similar pass.

-Tom



More information about the llvm-commits mailing list