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

[Davide Italiano via llvm-commits]

On Thu, Jul 21, 2016 at 5:50 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>
> 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.
>

I don't want to discuss this further, because I think it's silly, but
still, there's something I want to point out.
The major downside of porting all the tests now is that the time to
run the tests increases.
Porting the tests is generally trivial so I'm not entirely sure how
much it helps anticipating this work.

--
Davide