[llvm] r237226 - Change LoadAndStorePromoter to take ArrayRef instead of SmallVectorImpl&.

Pete Cooper peter_cooper at apple.com
Tue May 12 18:29:22 PDT 2015


Hey David

Wanted to get your opinion on improving this one further.

Ultimately I think it was a good thing to change the API as we didn’t need mutable ‘Instruction*’.  However, the only constructor ArrayRef has to do this conversion needs another ArrayRef to start with.

So what ends up happening is: SmallVectorImpl<T*> -> ArrayRef<T*> -> ArrayRef<const T*>.  Hence the makeArrayRef in this code.

What are your feelings on taking this existing code to handle the const conversion:


    /// Construct an ArrayRef<const T*> from ArrayRef<T*>. This uses SFINAE to
    /// ensure that only ArrayRefs of pointers can be converted.
    template <typename U>
    ArrayRef(const ArrayRef<U *> &A,
             typename std::enable_if<
                 std::is_convertible<U *const *, T const *>::value>::type* = 0)
      : Data(A.data()), Length(A.size()) {}

And applying it to the other constructors in ArrayRef, e.g., these 2:


    template<typename U>
    /*implicit*/ ArrayRef(const SmallVectorTemplateCommon<T, U> &Vec)
      : Data(Vec.data()), Length(Vec.size()) {
    }

    /// Construct an ArrayRef from a std::vector.
    template<typename A>
    /*implicit*/ ArrayRef(const std::vector<T, A> &Vec)
      : Data(Vec.data()), Length(Vec.size()) {}

I’d really like to be able to pass around containers of const pointers without extra code needed to convert from containers of mutable pointers.

Ideally, we could even apply similar tricks to SmallVector itself.

Thoughts?

Cheers,
Pete
> On May 12, 2015, at 6:12 PM, Pete Cooper <peter_cooper at apple.com> wrote:
> 
> Author: pete
> Date: Tue May 12 20:12:16 2015
> New Revision: 237226
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=237226&view=rev
> Log:
> Change LoadAndStorePromoter to take ArrayRef instead of SmallVectorImpl&.
> 
> The array passed to LoadAndStorePromoter's constructor was a constant reference to a SmallVectorImpl, which is just the same as passing an ArrayRef.
> 
> Also, the data in the array can be 'const Instruction*' instead of 'Instruction*'.  Its not possible to convert a SmallVectorImpl<T*> to SmallVectorImpl<const T*>, but ArrayRef does provide such a method.
> 
> Currently this added calls to makeArrayRef which should be a nop, but i'm going to kick off a discussion about improving ArrayRef to not need these.
> 
> Modified:
>    llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h
>    llvm/trunk/lib/Transforms/Scalar/LICM.cpp
>    llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>    llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp
>    llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp
> 
> Modified: llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h?rev=237226&r1=237225&r2=237226&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h (original)
> +++ llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h Tue May 12 20:12:16 2015
> @@ -14,6 +14,7 @@
> #ifndef LLVM_TRANSFORMS_UTILS_SSAUPDATER_H
> #define LLVM_TRANSFORMS_UTILS_SSAUPDATER_H
> 
> +#include "llvm/ADT/ArrayRef.h"
> #include "llvm/ADT/StringRef.h"
> #include "llvm/Support/Compiler.h"
> 
> @@ -135,7 +136,7 @@ protected:
>   SSAUpdater &SSA;
> 
> public:
> -  LoadAndStorePromoter(const SmallVectorImpl<Instruction*> &Insts,
> +  LoadAndStorePromoter(ArrayRef<const Instruction*> Insts,
>                        SSAUpdater &S, StringRef Name = StringRef());
>   virtual ~LoadAndStorePromoter() {}
> 
> 
> Modified: llvm/trunk/lib/Transforms/Scalar/LICM.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LICM.cpp?rev=237226&r1=237225&r2=237226&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/LICM.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/LICM.cpp Tue May 12 20:12:16 2015
> @@ -714,7 +714,8 @@ namespace {
>     }
> 
>   public:
> -    LoopPromoter(Value *SP, const SmallVectorImpl<Instruction *> &Insts,
> +    LoopPromoter(Value *SP,
> +                 ArrayRef<const Instruction *> Insts,
>                  SSAUpdater &S, SmallPtrSetImpl<Value *> &PMA,
>                  SmallVectorImpl<BasicBlock *> &LEB,
>                  SmallVectorImpl<Instruction *> &LIP, PredIteratorCache &PIC,
> @@ -920,7 +921,8 @@ bool llvm::promoteLoopAccessesToScalars(
>   // We use the SSAUpdater interface to insert phi nodes as required.
>   SmallVector<PHINode*, 16> NewPHIs;
>   SSAUpdater SSA(&NewPHIs);
> -  LoopPromoter Promoter(SomePtr, LoopUses, SSA, PointerMustAliases, ExitBlocks,
> +  LoopPromoter Promoter(SomePtr, makeArrayRef(LoopUses), SSA,
> +                        PointerMustAliases, ExitBlocks,
>                         InsertPts, PIC, *CurAST, *LI, DL, Alignment, AATags);
> 
>   // Set up the preheader to have a definition of the value.  It is the live-out
> 
> Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=237226&r1=237225&r2=237226&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Tue May 12 20:12:16 2015
> @@ -1088,7 +1088,8 @@ class AllocaPromoter : public LoadAndSto
>   SmallVector<DbgValueInst *, 4> DVIs;
> 
> public:
> -  AllocaPromoter(const SmallVectorImpl<Instruction *> &Insts, SSAUpdater &S,
> +  AllocaPromoter(ArrayRef<const Instruction *> Insts,
> +                 SSAUpdater &S,
>                  AllocaInst &AI, DIBuilder &DIB)
>       : LoadAndStorePromoter(Insts, S), AI(AI), DIB(DIB) {}
> 
> @@ -4418,7 +4419,7 @@ bool SROA::promoteAllocas(Function &F) {
>       DeadInsts.push_back(I);
>       enqueueUsersInWorklist(*I, Worklist, Visited);
>     }
> -    AllocaPromoter(Insts, SSA, *AI, DIB).run(Insts);
> +    AllocaPromoter(makeArrayRef(Insts), SSA, *AI, DIB).run(Insts);
>     while (!DeadInsts.empty())
>       DeadInsts.pop_back_val()->eraseFromParent();
>     AI->eraseFromParent();
> 
> Modified: llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp?rev=237226&r1=237225&r2=237226&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp Tue May 12 20:12:16 2015
> @@ -1052,7 +1052,7 @@ class AllocaPromoter : public LoadAndSto
>   SmallVector<DbgDeclareInst *, 4> DDIs;
>   SmallVector<DbgValueInst *, 4> DVIs;
> public:
> -  AllocaPromoter(const SmallVectorImpl<Instruction*> &Insts, SSAUpdater &S,
> +  AllocaPromoter(ArrayRef<Instruction*> Insts, SSAUpdater &S,
>                  DIBuilder *DB)
>     : LoadAndStorePromoter(Insts, S), AI(nullptr), DIB(DB) {}
> 
> 
> Modified: llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp?rev=237226&r1=237225&r2=237226&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp Tue May 12 20:12:16 2015
> @@ -322,12 +322,12 @@ Value *SSAUpdater::GetValueAtEndOfBlockI
> //===----------------------------------------------------------------------===//
> 
> LoadAndStorePromoter::
> -LoadAndStorePromoter(const SmallVectorImpl<Instruction*> &Insts,
> +LoadAndStorePromoter(ArrayRef<const Instruction*> Insts,
>                      SSAUpdater &S, StringRef BaseName) : SSA(S) {
>   if (Insts.empty()) return;
> 
> -  Value *SomeVal;
> -  if (LoadInst *LI = dyn_cast<LoadInst>(Insts[0]))
> +  const Value *SomeVal;
> +  if (const LoadInst *LI = dyn_cast<LoadInst>(Insts[0]))
>     SomeVal = LI;
>   else
>     SomeVal = cast<StoreInst>(Insts[0])->getOperand(0);
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150512/1cb4ee73/attachment.html>


More information about the llvm-commits mailing list