[PATCH] D42447: [LV][VPlan] Detect outer loops for explicit vectorization.

Diego Caballero via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 20 01:01:04 PDT 2018


dcaballe added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:8627
+  Function *F = L->getHeader()->getParent();
+  InterleavedAccessInfo IAI(PSE, L, DT, LI, LVL->getLAI());
+  LoopVectorizationCostModel CM(L, PSE, LI, LVL, *TTI, TLI, DB, AC, ORE, F,
----------------
hsaito wrote:
> hsaito wrote:
> > dcaballe wrote:
> > > @hsaito, there was a conflict with D45072 and I had to construct IAI here and carry over DT just for it. I wonder if it would be better to make IAI optional in CM to avoid this. It would make the code reuse easier. 
> > Feel free to change IAI into a pointer that can be nullptr. After all, InterleavedAccess is an optimization step that we should be able to skip. D45072 didn't get into that, but I was planning to do that while I clean up CostModel. I kept it as a reference simply because it was a reference in Legal. If you want me to do it, I can do it quick.
> > 
> The "Optimize" phase of the vectorizer most likely need DT anyway in a long run and thus having to carry over DT by itself is not a really bad thing. Part of the reason is your design choice of not making procesLoopInVPlanNativePath a member function of LoopVectorizePass class. For the time being, I think this part of the code can go in as is, i.e., without making IAI optional. I'll be touching CM code soon enough anyway.
> If you want me to do it, I can do it quick.

> I think this part of the code can go in as is, i.e., without making IAI optional. I'll be touching CM code soon enough anyway.

I'm ok with either of both. If you want to quickly fix it, I can wait for that patch. I won't be committing it until next week. Otherwise, you can remove this line later when you address it.

> Part of the reason is your design choice of not making procesLoopInVPlanNativePath a member function of LoopVectorizePass class.

I just tried to keep these changes away from LoopVectorize.h but the bunch of parameter is certainly inconvenient. If you think it's better, I can just make it member. Please, let me know what you think.


https://reviews.llvm.org/D42447





More information about the llvm-commits mailing list