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

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 16 13:49:44 PDT 2016


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


More information about the llvm-commits mailing list