[PATCH] D45493: [Polly][NewPM] Port IslAst, ScopDetect, and ScopInfo lit testsuites over to the new PM

Philip Pfaffe via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 16 04:04:16 PDT 2018


philip.pfaffe added a comment.

In https://reviews.llvm.org/D45493#1066480, @Meinersbur wrote:

> In https://reviews.llvm.org/D45493#1064052, @philip.pfaffe wrote:
>
> > In https://reviews.llvm.org/D45493#1063228, @Meinersbur wrote:
> >
> > > Instead of adding `-aa-pipeline=basic-aa` everywhere, did you consider adding it to `%loadPolly`, or only add it if the test otherwise fails?
> >
> >
> > I can add it to %loadPolly.  I'm hesitant to make this opt-in, though, since I fear the tests could get very flaky.
>
>
> Can you elaborate on this?


In a legacy PM pipeline, basic-aa is always the default. I.e., if you do AA queries without specifying any AA passes, you always get basic-aa results. In the new PM, that's not true. So i think it should be present in the testcases. I'm happy to add it to `%loadPolly', though!

>>> This will double the time a check-polly run takes, which I unfortunately cannot get too excited about. For new test cases, do we then also have to consider the new pass manager?
>> 
>> New test cases should be written for the new pass manager. Possibly, we should require future changes to be new-pm only? I don't think it makes sense to invest in a legacy codebase.
>> 
>>> My suggestion is to wait with this change until we want to make the new PM the main implementation, instead of maintaining two PMs in parallel. The script can be applied anytime, and even added to the repository.
>> 
>> I can agree with that, but I think we should make the switch asap (since this is mostly a change in policy).
> 
> I'd go with when clang/LLVM itself makes the switch.

I'd prefer to have coverage before that. But actually this feels like we should have broader discussion on whether or when we should deprecate legacy PM in Polly and should expect contributors to code against the new PM instead.


Repository:
  rPLO Polly

https://reviews.llvm.org/D45493





More information about the llvm-commits mailing list