[patch] simplifyCFG for review

[Tom Stellard]

On Sat, Jul 27, 2013 at 05:01:15PM +0200, Christian König wrote:
> Hi guys,
> 
> Am 27.07.2013 10:56, schrieb Andrew Trick:
> >
> >On Jul 27, 2013, at 1:30 AM, Chandler Carruth
> ><chandlerc at google.com <mailto:chandlerc at google.com>> wrote:
> >
> >>
> >>On Sat, Jul 27, 2013 at 1:07 AM, Andrew Trick <atrick at apple.com
> >><mailto:atrick at apple.com>> wrote:
> >>
> >>>    (1) In terms of code organization, these anti-canonical,
> >>>    target-selected transforms should live somewhere else. I kept
> >>>    quiet becase I hadn't come up with an alternative, but we can do
> >>>    that now. OptimizeCFG.cpp?
> >>
> >>    It would be even more self-evident to group all of these type of
> >>    branch avoidance utils into a FlattenCFG package. So I'm changing
> >>    my vote to FlattenCFG.cpp.
> >>
> >>
> >>To focus on the immediate issue: I agree.
> >>
> >>Design wise, I would suggest one step further: I think that
> >>CFG-flattening of this form is somewhat specialized and we
> >>should just build a nice specialized set of optimizations
> >>targetting that use case, and have the targets add those passes
> >>from their target machine rather than monkey with the general
> >>purpose PMB. We can't easily get this right in the PMB because
> >>of the silly way CGSCC stuff is defined, and I think that this
> >>is likely to be best as a late-stage CFG pass anyways not unlike
> >>LSR, etc. I'm not sure that it really has anything to do with
> >>SimplifyCFG (or OptimizeCFG, which I've begun to think is
> >>somewhat likely to make more sense in MI w/ register pressure
> >>and critical path info). So, my vote is for a more targeted tool
> >>in the toolbox.
> >
> >Mei,
> >
> >You seem to have a reason for running SimplifyParallelAndOr after
> >the basic cleanup transforms but before branch simplification.
> >Whereas MergeIfRegion runs only after all other simplifications.
> >That does seem intuitive to me, but I wonder if it is absolutely
> >necessary. Can you illustrate with an example or two why the
> >simplifications need to be interleaved this way? Are you just
> >trying to avoid the need for another round of iteration in the
> >SimplifyCFGPass driver? I think the direction of design
> >refactoring depends on it.
> >
> 
> I strongly agree with Chandlers opinion that those target dependent
> optimization passes shouldn't be added by any target independent
> pass manager.
> 
> I haven't followed the full discussion, but to me it looked like one
> of the main arguments against running those passes later were
> concerns about the compile time because additional to those CFG
> modifications the R600 target also requires that we inline ALL
> functions. This inlining of all functions is necessary because the
> R600 architecture doesn't have a conventional stack and so can't
> make most kind of subroutine calls.
> 
> I strongly suspect that this (target specific) inlining of all
> functions is done by the target independent function inliner using
> something like the "always inline" flag on every function or
> something like this.
>

This is probably how we should be doing it, but for r600g/radeonsi we
use the regular inliner pass and specify a very large cost threshold.
Marking everything as "always inline" and using the always inline pass,
sounds like a much better solution, even if it is still a bit wrong.

-Tom

> I just wanted to note that doing it like this is actually a bit
> wrong, cause we are using a target independent optimization feature
> to fulfill our target dependent requirements. The better approach
> would be to inline all functions in a target specific pass right
> after the target specific optimizations and before CFG
> structurization and code generation.
> 
> This would also allow us to handle things like inlining of recursive
> function, which is necessary for R600 but also something no halve
> way sane target independent optimizer would do.
> 
> Just some thoughts about this topic,
> Christian.
> 
> >-Andy
> >
> >
> >_______________________________________________
> >llvm-commits mailing list
> >llvm-commits at cs.uiuc.edu
> >http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits