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