<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hey David<div class=""><br class=""></div><div class="">Wanted to get your opinion on improving this one further.</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">So what ends up happening is: SmallVectorImpl<T*> -> ArrayRef<T*> -> ArrayRef<const T*>. Hence the makeArrayRef in this code.</div><div class=""><br class=""></div><div class="">What are your feelings on taking this existing code to handle the const conversion:</div><div class=""><br class=""></div><div class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>/// Construct an ArrayRef<const T*> from ArrayRef<T*>. This uses SFINAE to</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>/// ensure that only ArrayRefs of pointers can be converted.</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(187, 44, 162);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>template<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> <</span>typename<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> U></span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> ArrayRef(<span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">const</span> <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">ArrayRef</span><U *> &A,</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">typename</span> <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">std</span>::<span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">enable_if</span><</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">std</span>::<span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">is_convertible</span><U *<span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">const</span> *, T <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">const</span> *>::value>::type* = <span style="font-variant-ligatures: no-common-ligatures; color: #272ad8" class="">0</span>)</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> : <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">Data</span>(A.data()), <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">Length</span>(A.size()) {}</div></div><div class=""><br class=""></div><div class="">And applying it to the other constructors in ArrayRef, e.g., these 2:</div><div class=""><br class=""></div><div class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(187, 44, 162);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>template<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""><</span>typename<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> U></span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #008400" class="">/*implicit*/</span> ArrayRef(<span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">const</span> <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">SmallVectorTemplateCommon</span><T, U> &Vec)</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> : <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">Data</span>(Vec.data()), <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">Length</span>(Vec.size()) {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> }</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>/// Construct an ArrayRef from a std::vector.</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(187, 44, 162);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>template<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""><</span>typename<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> A></span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #008400" class="">/*implicit*/</span> ArrayRef(<span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">const</span> <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">std</span>::<span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">vector</span><T, A> &Vec)</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> : <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">Data</span>(Vec.data()), <span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">Length</span>(Vec.size()) {}</div></div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">Ideally, we could even apply similar tricks to SmallVector itself.</div><div class=""><br class=""></div><div class="">Thoughts?</div><div class=""><br class=""></div><div class="">Cheers,</div><div class="">Pete<br class=""><div><blockquote type="cite" class=""><div class="">On May 12, 2015, at 6:12 PM, Pete Cooper <<a href="mailto:peter_cooper@apple.com" class="">peter_cooper@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">Author: pete<br class="">Date: Tue May 12 20:12:16 2015<br class="">New Revision: 237226<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=237226&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=237226&view=rev</a><br class="">Log:<br class="">Change LoadAndStorePromoter to take ArrayRef instead of SmallVectorImpl&.<br class=""><br class="">The array passed to LoadAndStorePromoter's constructor was a constant reference to a SmallVectorImpl, which is just the same as passing an ArrayRef.<br class=""><br class="">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.<br class=""><br class="">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.<br class=""><br class="">Modified:<br class=""> llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h<br class=""> llvm/trunk/lib/Transforms/Scalar/LICM.cpp<br class=""> llvm/trunk/lib/Transforms/Scalar/SROA.cpp<br class=""> llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp<br class=""> llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp<br class=""><br class="">Modified: llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h?rev=237226&r1=237225&r2=237226&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h?rev=237226&r1=237225&r2=237226&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h (original)<br class="">+++ llvm/trunk/include/llvm/Transforms/Utils/SSAUpdater.h Tue May 12 20:12:16 2015<br class="">@@ -14,6 +14,7 @@<br class=""> #ifndef LLVM_TRANSFORMS_UTILS_SSAUPDATER_H<br class=""> #define LLVM_TRANSFORMS_UTILS_SSAUPDATER_H<br class=""><br class="">+#include "llvm/ADT/ArrayRef.h"<br class=""> #include "llvm/ADT/StringRef.h"<br class=""> #include "llvm/Support/Compiler.h"<br class=""><br class="">@@ -135,7 +136,7 @@ protected:<br class=""> SSAUpdater &SSA;<br class=""><br class=""> public:<br class="">- LoadAndStorePromoter(const SmallVectorImpl<Instruction*> &Insts,<br class="">+ LoadAndStorePromoter(ArrayRef<const Instruction*> Insts,<br class=""> SSAUpdater &S, StringRef Name = StringRef());<br class=""> virtual ~LoadAndStorePromoter() {}<br class=""><br class=""><br class="">Modified: llvm/trunk/lib/Transforms/Scalar/LICM.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LICM.cpp?rev=237226&r1=237225&r2=237226&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LICM.cpp?rev=237226&r1=237225&r2=237226&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Transforms/Scalar/LICM.cpp (original)<br class="">+++ llvm/trunk/lib/Transforms/Scalar/LICM.cpp Tue May 12 20:12:16 2015<br class="">@@ -714,7 +714,8 @@ namespace {<br class=""> }<br class=""><br class=""> public:<br class="">- LoopPromoter(Value *SP, const SmallVectorImpl<Instruction *> &Insts,<br class="">+ LoopPromoter(Value *SP,<br class="">+ ArrayRef<const Instruction *> Insts,<br class=""> SSAUpdater &S, SmallPtrSetImpl<Value *> &PMA,<br class=""> SmallVectorImpl<BasicBlock *> &LEB,<br class=""> SmallVectorImpl<Instruction *> &LIP, PredIteratorCache &PIC,<br class="">@@ -920,7 +921,8 @@ bool llvm::promoteLoopAccessesToScalars(<br class=""> // We use the SSAUpdater interface to insert phi nodes as required.<br class=""> SmallVector<PHINode*, 16> NewPHIs;<br class=""> SSAUpdater SSA(&NewPHIs);<br class="">- LoopPromoter Promoter(SomePtr, LoopUses, SSA, PointerMustAliases, ExitBlocks,<br class="">+ LoopPromoter Promoter(SomePtr, makeArrayRef(LoopUses), SSA,<br class="">+ PointerMustAliases, ExitBlocks,<br class=""> InsertPts, PIC, *CurAST, *LI, DL, Alignment, AATags);<br class=""><br class=""> // Set up the preheader to have a definition of the value. It is the live-out<br class=""><br class="">Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=237226&r1=237225&r2=237226&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=237226&r1=237225&r2=237226&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)<br class="">+++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Tue May 12 20:12:16 2015<br class="">@@ -1088,7 +1088,8 @@ class AllocaPromoter : public LoadAndSto<br class=""> SmallVector<DbgValueInst *, 4> DVIs;<br class=""><br class=""> public:<br class="">- AllocaPromoter(const SmallVectorImpl<Instruction *> &Insts, SSAUpdater &S,<br class="">+ AllocaPromoter(ArrayRef<const Instruction *> Insts,<br class="">+ SSAUpdater &S,<br class=""> AllocaInst &AI, DIBuilder &DIB)<br class=""> : LoadAndStorePromoter(Insts, S), AI(AI), DIB(DIB) {}<br class=""><br class="">@@ -4418,7 +4419,7 @@ bool SROA::promoteAllocas(Function &F) {<br class=""> DeadInsts.push_back(I);<br class=""> enqueueUsersInWorklist(*I, Worklist, Visited);<br class=""> }<br class="">- AllocaPromoter(Insts, SSA, *AI, DIB).run(Insts);<br class="">+ AllocaPromoter(makeArrayRef(Insts), SSA, *AI, DIB).run(Insts);<br class=""> while (!DeadInsts.empty())<br class=""> DeadInsts.pop_back_val()->eraseFromParent();<br class=""> AI->eraseFromParent();<br class=""><br class="">Modified: llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp?rev=237226&r1=237225&r2=237226&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp?rev=237226&r1=237225&r2=237226&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp (original)<br class="">+++ llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp Tue May 12 20:12:16 2015<br class="">@@ -1052,7 +1052,7 @@ class AllocaPromoter : public LoadAndSto<br class=""> SmallVector<DbgDeclareInst *, 4> DDIs;<br class=""> SmallVector<DbgValueInst *, 4> DVIs;<br class=""> public:<br class="">- AllocaPromoter(const SmallVectorImpl<Instruction*> &Insts, SSAUpdater &S,<br class="">+ AllocaPromoter(ArrayRef<Instruction*> Insts, SSAUpdater &S,<br class=""> DIBuilder *DB)<br class=""> : LoadAndStorePromoter(Insts, S), AI(nullptr), DIB(DB) {}<br class=""><br class=""><br class="">Modified: llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp?rev=237226&r1=237225&r2=237226&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp?rev=237226&r1=237225&r2=237226&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp (original)<br class="">+++ llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp Tue May 12 20:12:16 2015<br class="">@@ -322,12 +322,12 @@ Value *SSAUpdater::GetValueAtEndOfBlockI<br class=""> //===----------------------------------------------------------------------===//<br class=""><br class=""> LoadAndStorePromoter::<br class="">-LoadAndStorePromoter(const SmallVectorImpl<Instruction*> &Insts,<br class="">+LoadAndStorePromoter(ArrayRef<const Instruction*> Insts,<br class=""> SSAUpdater &S, StringRef BaseName) : SSA(S) {<br class=""> if (Insts.empty()) return;<br class=""><br class="">- Value *SomeVal;<br class="">- if (LoadInst *LI = dyn_cast<LoadInst>(Insts[0]))<br class="">+ const Value *SomeVal;<br class="">+ if (const LoadInst *LI = dyn_cast<LoadInst>(Insts[0]))<br class=""> SomeVal = LI;<br class=""> else<br class=""> SomeVal = cast<StoreInst>(Insts[0])->getOperand(0);<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""></div></body></html>