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

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

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.



================
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
----------------
This sounds bad. If they have common signature we could have linking issues.

Regardless, we should keep inline functions to cpp files, not headers.


================
Comment at: lib/Analysis/VectorUtils.cpp:587
+#define LV_NAME "loop-vectorize"
+#define DEBUG_TYPE LV_NAME
+
----------------
This is not strictly true for all users, which are many.

    $ grep -riI VectorUtils.h
    lib/Transforms/InstCombine/InstCombineVectorOps.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Transforms/Scalar/Scalarizer.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Transforms/Vectorize/SLPVectorizer.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Transforms/Vectorize/LoopVectorize.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Analysis/InstructionSimplify.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Analysis/VectorUtils.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Analysis/LoopAccessAnalysis.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Target/X86/X86InterleavedAccess.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Target/ARM/ARMISelLowering.cpp:#include "llvm/Analysis/VectorUtils.h"
    lib/Target/AArch64/AArch64ISelLowering.cpp:#include "llvm/Analysis/VectorUtils.h"
    include/llvm/Analysis/TargetTransformInfoImpl.h:#include "llvm/Analysis/VectorUtils.h"



https://reviews.llvm.org/D49488