This is an archive of the discontinued LLVM Phabricator instance.

[NFC][LV] Move InterleaveInfo from Legal to CostModel
ClosedPublic

Authored by hsaito on Mar 29 2018, 5:25 PM.

Details

Summary

Another clean up, following D43208.

Interleaved memory access analysis/optimization has nothing to do with vectorization legality. It doesn't really belong there. On the other hand, cost model certainly has to know about it.

In principle, vectorization should proceed like Legality ==> Optimization ==> CostModel ==> CodeGen, and this change just does that,
by moving the interleaved access analysis/decision out of Legal, and run it just before CostModel object is created.

After this, I can move LoopVectorizationLegality and Hints/Requirements classes into it's own header file, making it shareable within Transform tree. I have the patch already but I don't want to mix with this change. Eventual goal is to move to Analysis tree, but I first need to move RecurrenceDescriptor/InductionDescriptor from Transform/Util/LoopUtil.* to Analysis.

Diff Detail

Event Timeline

hsaito created this revision.Mar 29 2018, 5:25 PM
hsaito added a comment.Apr 6 2018, 7:36 PM

Ping. I'll soon have a patch for moving RecurrenceDescriptor/InductionDescriptor classes to Analysis tree, followed by another patch to move LoopVectorizationLegality and few others to Analysis tree.

Thanks,
Hideki

I just submitted D45420 for review. That patch moves RecurrenceDescriptor and InductionDescriptor to Analysis tree.
Plain is to move LoopVectorizationLegality out of LoopVectorize.cpp into Analysis tree after D45072 and D45420 go in.

rengolin accepted this revision.Apr 8 2018, 3:47 PM

Hi Hideki,

Really sorry about the delay. This change makes a lot of sense and should be totally NFC. LGTM. Thanks!

This revision is now accepted and ready to land.Apr 8 2018, 3:47 PM
This revision was automatically updated to reflect the committed changes.
hsaito added a comment.Apr 9 2018, 6:24 PM

This patch eliminated the last use of TTI from LoopVectorizationLegality --- which led to a buildbot failure (fixed in rL329649). Makes sense since vectorization legality doesn't really depends on the target.
In the future, we probably want to revisit TLI here as well.