[PATCH] D45552: [NFC][LV][LoopUtil] Move LoopVectorizationLegality to its own file

Renato Golin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 30 10:03:16 PDT 2018


rengolin added a comment.

In https://reviews.llvm.org/D45552#1082936, @hsaito wrote:

> For example, in loop fusion scenarios, doing this is not a rocket science.


Right, this is still loop transformation, so staying inside Transform still makes sense.

I'm not disagreeing with you on the topics:

1. It does look like an analysis pass
2. Other passes could profit

However, so far, the other passes are all in the transform directory.

I agree we need to split "what" can be done from "how we'll do it" and I agree we need to make `LoopUtils` more generic. I was just trying to be clear on why we shouldn't move to `include/Analysis` yet.

> This is a chicken-egg problem. In my opinion, since vectorizer has so much performance impact, vectorizer's analysis aspect should be made available for others for "easier reuse".

Not yet, since we can already do that, while keeping the headers inside `Transform`.

We shouldn't think of the `InnerLoopVectorizer` as the "loop vectoriser". VPlan, SLP and other loop transformations are all part of the "loop transformations" which will all need to reuse the same analysis.

So, moving things our of the `ILV` is great and we should do it ASAP!

Moving them away from `Transform` will need a reason, of which we don't have one yet.

> Next thing I'd like to move up to a higher ground is CostModel. I'll be sure to send in an RFC for that.

Great! We'll need to understand how that ties with the existing TTI hooks and TableGen's scheduling information.

For a long time I wanted to have a cost model with peephole analysis, or at least pattern-matching sequences, but the TTI hooks aren't too good for that and I never had the time to refactor it.

Sorry if I sounded terse recently, I was just trying to explain the reasons, not flog the dead horse. :)

Thanks!
--renato


Repository:
  rL LLVM

https://reviews.llvm.org/D45552





More information about the llvm-commits mailing list