[llvm-dev] 6 separate instances of static getPointerOperand(). Time to consolidate?

[Justin Bogner via llvm-dev]

"Saito, Hideki via llvm-dev" <llvm-dev at lists.llvm.org> writes:
> LLVM friends,
>
> I'm currently trying to make LoopVectorizationLegality class in
> Transform/Vectorize/LoopVectorize.cpp more modular and eventually move
> it to Analysis directory tree. It uses several file scope helper
> functions that do not really belong to LoopVectorize. Let me start
> from getPointerOperand(). Within LLVM, there are five other similar
> functions defined.
>
> I think it's time to define/declare this function in one place.
>
> Define in include/llvm/IR/Instructions.h as an inline function?
> Declare in include/llvm/IR/Instructions.h and define in lib/IR/Instructions.cpp?
> New files? (e.g., IR/InstructionUtils.h and .cpp?)
>                Other suggesctions?
>
> I suggest taking the implementation from Analysis/Delinearization.cpp,
> and create IR/InstructionUtils.h and .cpp. They can then be used to
> consolidate other Instruction helper functions to one place.

What other instruction helper functions do you have in mind? Names like
"Utils" worry me because they generally turn into a dumping ground for
things that people didn't bother to think about where belong, which in
turn makes organizing those things later difficult.

> Objections? Alternatives?
>
> Thanks,
> Hideki
> -----------
> Relevant background info
> http://lists.llvm.org/pipermail/llvm-dev/2018-January/120164.html
> if anyone is curious about my cleanup work.
>
> ------------- Analysis/Delinearization.cpp
>
> static Value *getPointerOperand(Instruction &Inst) {
>   if (LoadInst *Load = dyn_cast<LoadInst>(&Inst))
>     return Load->getPointerOperand();
>   else if (StoreInst *Store = dyn_cast<StoreInst>(&Inst))
>     return Store->getPointerOperand();
>   else if (GetElementPtrInst *Gep = dyn_cast<GetElementPtrInst>(&Inst))
>     return Gep->getPointerOperand();
>   return nullptr;
> }
>
> ------------- Analysis/DependenceAnalysis.cpp
>
> static
> Value *getPointerOperand(Instruction *I) {
>   if (LoadInst *LI = dyn_cast<LoadInst>(I))
>     return LI->getPointerOperand();
>   if (StoreInst *SI = dyn_cast<StoreInst>(I))
>     return SI->getPointerOperand();
>   llvm_unreachable("Value is not load or store instruction");
>   return nullptr;
> }
>
> ------------- Analysis/LoopAccessAnalysis.cpp
>
> /// Take the pointer operand from the Load/Store instruction.
> /// Returns NULL if this is not a valid Load/Store instruction.
> static Value *getPointerOperand(Value *I) {
>   if (auto *LI = dyn_cast<LoadInst>(I))
>     return LI->getPointerOperand();
>   if (auto *SI = dyn_cast<StoreInst>(I))
>     return SI->getPointerOperand();
>   return nullptr;
> }
>
> ------------ Transforms/Scalar/EarlyCSE.cpp    class ParseMemoryInst
>
>     Value *getPointerOperand() const {
>       if (IsTargetMemInst) return Info.PtrVal;
>       if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
>         return LI->getPointerOperand();
>       } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
>         return SI->getPointerOperand();
>       }
>       return nullptr;
>     }
>
> ------------ Transforms/Vectorize/LoadStoreVectorizer.cpp     class Vectorizer
>
> Value *Vectorizer::getPointerOperand(Value *I) const {
>   if (LoadInst *LI = dyn_cast<LoadInst>(I))
>     return LI->getPointerOperand();
>   if (StoreInst *SI = dyn_cast<StoreInst>(I))
>     return SI->getPointerOperand();
>   return nullptr;
> }
>
> ---------- Transforms/Vectorize/LoopVectorize.cpp
>
> // FIXME: The following helper functions have multiple implementations
> // in the project. They can be effectively organized in a common Load/Store
> // utilities unit.
> /// A helper function that returns the pointer operand of a load or store
> /// instruction.
> static Value *getPointerOperand(Value *I) {
>   if (auto *LI = dyn_cast<LoadInst>(I))
>     return LI->getPointerOperand();
>   if (auto *SI = dyn_cast<StoreInst>(I))
>     return SI->getPointerOperand();
>   return nullptr;
> }
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev