[PATCH] D49488: [LV] Move InterleaveGroup and InterleavedAccessInfo to VectorUtils.h (NFC)

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 18 08:49:25 PDT 2018


fhahn added a comment.

In https://reviews.llvm.org/D49488#1166638, @rengolin wrote:

> I like the idea of the interleave analysis to be at a higher ground, but we have to be careful with LV-specific logic and only hoist what it truly generic.
>
> Some comments inline.


Thanks for having a look so quickly! Maybe there is a better place to put it, maybe we should keep it local to lib/Transforms/Vectorize/? I initially put it in VectorUtils.h, because I wanted to avoid creating unnecessary new files (and splitting things up unnecessarily), but given that VectorUtils.h is used in quite a few places, I am happy to put it wherever it would fit best :)



================
Comment at: include/llvm/Analysis/VectorUtils.h:510
+
+// FIXME: The following helper functions have multiple implementations
+// in the project. They can be effectively organized in a common Load/Store
----------------
rengolin wrote:
> This sounds bad. If they have common signature we could have linking issues.
> 
> Regardless, we should keep inline functions to cpp files, not headers.
Yep I did some digging but could not really pin down other users where they would be helpful. As indicated in the comment, neither LoopVectorize.cpp nor VectorUtils.h seems like appropriate places. However I am unsure what the ideal place would be?


https://reviews.llvm.org/D49488





More information about the llvm-commits mailing list