[llvm] r272505 - [PM] Port ReversePostOrderFunctionAttrs to the new PM

[Xinliang David Li via llvm-commits]

Davide, as Danny noticed, I think the main troublesome patch is r272595.
There is no need to eliminate that class and statiize all the methods.
Instead, what is needed is to move legacy pass specific members out of
class MergedLoadStoreMotion and keep the original class as the
'transformation class' that can be shared.

Can you revert that change and do the above suggested refactoring -- that
will only take a few lines, I think.

thanks,

David


On Thu, Jun 16, 2016 at 1:49 PM, Davide Italiano via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> On Thu, Jun 16, 2016 at 9:48 AM, Daniel Berlin <dberlin at dberlin.org>
> wrote:
> > Hey guys:
> >
> > I'm a little concerned how some of these passes are being converted.
> >
> > For example, mergedloadstoremotion was entirely turned into a bunch of
> > static helpers that pass around lots of what was previously simple class
> > state.
> >
> > With no review, or discussion.
> >
> > I find it *much* harder to understand, and in fact, with the memoryssa
> > version coming, it would mean tons of functions now get passed 4 or 5
> random
> > things like AA, DT, MD, MSSA, etc.
> >
> > This doesn't make a lot of sense, and it could have been easily avoided
> > entirely. The simplest refactoring would have been to modify the existing
> > class a bit so it looks closer to the classes we use for utils, pass the
> > right pieces in the constructor, and leave the rest alone.
> >
> > In fact, i've asked for that particular set of patches to be reverted.
> >
> > I'm not opposed to simply committing them when they convert one thing to
> a
> > static helper or whatever.
> > But i think if what is happening is wholesale conversion of classes to
> > statics, etc, either it needs review, or we shouldn't be doing it that
> way.
> >
> > Because, IMHO, in at least that case, it ended up *much* worse than the
> > original pass.
> >
>
> I missed this thread (only saw the comment on the original patch).
> The pass was reverted. Again, I recommend you to take a look at the
> existing passes ported (at least by me) and let me know if there's
> something you don't like or whatever so that I can try to fix/revert.
>
> --
> Davide
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160616/fe44fad2/attachment.html>