[PATCH] D26869: [LV] Add flag for ignoring target info

Adam Nemet via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 22 23:12:18 PST 2016


anemet added a comment.

In https://reviews.llvm.org/D26869#603907, @mkuper wrote:

> In https://reviews.llvm.org/D26869#603890, @anemet wrote:
>
> > Hi Matt,
> >
> > You should probably circulate this more widely on llvm-dev.  This is a new policy on how one should write tests going under test/Transform/LoopVectorize.
> >
> > Thanks,
> > Adam
>
>
> It's more of a bugfix in the current policy - we used to have "target independent" tests because providing a manual UF and VF would, in effect, mean we'd make no cost model queries that affect the output. That was basically dumb luck - conceptually, tests for passes that rely on TTI need to either specify a target, or somehow be forced to be target-independent.
>
> But I agree, it may be worth circulating more widely, since this problem probably exists outside the LV vectorizer, so we may want a more principled way to do it.


Also there is actual shift of how tests should be written now.  I think that we all got used to writing these tests by forcing the vectorization and interleave factors.

> Do you have an opinion on the concept itself, other than "circulate more widely"?

I haven't been following the review closely where this came up so I didn't understand the rational and the summary does not really explain how the current practice breaks down.   I am assuming we want to write target-indepedent tests but the vectorizer needs a cost model.

Why aren't we adding a forcing flag for this feature as well, just like forcing the vectorization/interleave factors?  That may make the test more explicit rather than using the default TTI.  To me that would be the generalization of the current concept.

I guess what I am missing to get convinced is a run-down of the alternatives.  I know this was discussed in the other review but there is no reference here.  It's good to document these things for posterity.


https://reviews.llvm.org/D26869





More information about the llvm-commits mailing list