[PATCH] D22648: [PM] Port NaryReassociate to the new PM

[Sean Silva via llvm-commits]

On Thu, Jul 21, 2016 at 2:46 PM, Wei Mi <wmi at google.com> wrote:

> wmi added inline comments.
>
> ================
> Comment at: test/Transforms/NaryReassociate/nary-mul.ll:2
> @@ -1,2 +1,3 @@
>  ; RUN: opt < %s -nary-reassociate -S | FileCheck %s
> +; RUN: opt < %s -passes='nary-reassociate' -S | FileCheck %s
>
> ----------------
> davidxl wrote:
> > How about other tests in the same dir?
> I saw the guildeline in https://reviews.llvm.org/rL272505 said one test
> changed was enough. I just tried other tests and they passed. Do you want
> me to include them?
>

To clarify this, I was trying to condense existing practice in what I wrote
there (for example, it also says to rename to "LegacyPass" which isn't
really necessary in most cases). If it is easy to port all the tests then I
tend to think now that it is a good idea to do so.

One issue is that sometimes some tests depend on other unported passes
(this should be less common now that we have so many ported). Also,
sometimes the change cannot be easily done mechanically (this is true for
tests that use multiple passes). In those cases I think it is reasonable to
port only one or some tests in the initial patch.

David is right that the tests will all need to be changed anyway for the
transition, so if it is easy to port more tests, that will help down the
road.

-- Sean Silva


>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D22648
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160721/2a7ccad2/attachment.html>