[LLVMdev] Elsa and LLVM and LLVM submissions

David Greene dag at cray.com
Mon Dec 17 11:42:04 PST 2007


On Monday 17 December 2007 13:08, Devang Patel wrote:
> I used &Idx[0]. In future, please avoid tabs in your patch.
>
> I applied your patch.
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20071217/056403
>.html -
> Devang

Devang,

This patch is dangerous:

+  template<typename InputIterator>
+  Value *CreateGEP(Value *Ptr, InputIterator IdxBegin, 
+                   InputIterator IdxEnd, const char *Name = "") {
+    
+    if (Constant *PC = dyn_cast<Constant>(Ptr)) {
+      // Every index must be constant.
+      InputIterator i;
+      for (i = IdxBegin; i < IdxEnd; ++i) 
+        if (!dyn_cast<Constant>(*i))
+          break;
+      if (i == IdxEnd)
+        return ConstantExpr::getGetElementPtr(PC, &IdxBegin[0], IdxEnd - 
IdxBegin);

&IdxBegin[0] is a very bad idea.  If idxBegin == IdxEnd this will fault at
best.

The patch also assumes InputIterator is a random-access iterator, which
is true in this case but isn't guaranteed.

It looks like we need some new ConstantExpr::getGetElementPtr interfaces.
It should be a simple matter to copy the implementation of 

Constant *ConstantExpr::getGetElementPtr(Constant *C, Value* const *Idxs,
                                         unsigned NumIdx)

and pass iterators instread.  The implementation would simply forward the 
iterators to GetElementPtrInst::getIndexedType which already has the
necessary interfaces that ensure the iterators are random-access.

                                      -Dave




More information about the llvm-dev mailing list