thr3ads.net - llvm dev - [LLVMdev] GEP::getIndexValid() with other iterators [Jul 2008]

If this information is useful, please help other people find it:
Share via:

Chris Lattner

2008-Jul-23 07:00 UTC

[LLVMdev] GEP::getIndexValid() with other iterators

On Jul 22, 2008, at 11:54 PM, Matthijs Kooijman wrote:
> Hi Chris,
>
>
>> I'd prefer to not turn this into a template.  Why not just define a
>> version that takes an array of uint64_t's or something like that?
> because I want to be able to pass in iterators. I could define a  
> version that
> takes std<uint64_t>::iterators, but next thing we know, we also need
> them for
> lists, SmallVectors, etc. That's why one of the original  
> getIndexedType
> methods is a template, and that's why I think it makes sense to make  
> another
> one a template.
What flavor of iterators do you want to pass in?  vector or  
smallvector?  If so, a pointer to the first element + extents is fine.
> Any particular objections to this? Is the code size increase a  
> problem?
> AFAICS, in cases where you need this method, it will be a tradeoff  
> between
> speed (having to iterate all your indices and create a new list with  
> the
> Value* versions) and code size (having a version of  
> getIndexedPointer that can
> handle your particular flavour of iterator).
My basic objection is that I don't like tons of code in header files.   
It obfuscates the header and slows down compile times (of llvm itself)

-Chris

Matthijs Kooijman

2008-Jul-23 15:43 UTC

head link

[LLVMdev] GEP::getIndexValid() with other iterators

Hi Chris,
> What flavor of iterators do you want to pass in?  vector or  
> smallvector?  If so, a pointer to the first element + extents is fine.I was passing a std::vector in. And you are quite right, first element + size
works like a charm.

Since I am using a vector of uint64_t's instead of Value*, I did need to add
an extra version of getIndexedType(), taking a uint64_t* instead of Value**. 
> My basic objection is that I don't like tons of code in header files.
> It obfuscates the header and slows down compile times (of llvm itself)Ah, that is even a better reason :-)

I did use a template function to prevent duplicating code, but since it is
only used in the implementation of two getIndexedType() methods, it is in the
cpp file.

So, is this better?

Gr.

Matthijs

Index: lib/VMCore/Instructions.cpp
==================================================================---
lib/VMCore/Instructions.cpp	(revision 53716)
+++ lib/VMCore/Instructions.cpp	(working copy)
@@ -1071,12 +1071,16 @@
 // getIndexedType - Returns the type of the element that would be loaded with
 // a load instruction with the specified parameters.
 //
+// The Idxs pointer should point to a continuous piece of memory containing the
+// indices, either as Value* or uint64_t.
+//
 // A null type is returned if the indices are invalid for the specified
 // pointer type.
 //
-const Type* GetElementPtrInst::getIndexedType(const Type *Ptr,
-                                              Value* const *Idxs,
-                                              unsigned NumIdx) {
+template <typename IndexTy>
+static const Type* getIndexedTypeInternal(const Type *Ptr,
+                                  IndexTy const *Idxs,
+                                  unsigned NumIdx) {
   const PointerType *PTy = dyn_cast<PointerType>(Ptr);
   if (!PTy) return 0;   // Type isn't a pointer type!
   const Type *Agg = PTy->getElementType();
@@ -1089,7 +1093,7 @@
   for (; CurIdx != NumIdx; ++CurIdx) {
     const CompositeType *CT = dyn_cast<CompositeType>(Agg);
     if (!CT || isa<PointerType>(CT)) return 0;
-    Value *Index = Idxs[CurIdx];
+    IndexTy Index = Idxs[CurIdx];
     if (!CT->indexValid(Index)) return 0;
     Agg = CT->getTypeAtIndex(Index);
 
@@ -1103,6 +1107,18 @@
   return CurIdx == NumIdx ? Agg : 0;
 }
 
+const Type* GetElementPtrInst::getIndexedType(const Type *Ptr,
+                                              Value* const *Idxs,
+                                              unsigned NumIdx) {
+  return getIndexedTypeInternal(Ptr, Idxs, NumIdx);
+}
+
+const Type* GetElementPtrInst::getIndexedType(const Type *Ptr,
+                                              uint64_t const *Idxs,
+                                              unsigned NumIdx) {
+  return getIndexedTypeInternal(Ptr, Idxs, NumIdx);
+}
+
 const Type* GetElementPtrInst::getIndexedType(const Type *Ptr, Value *Idx) {
   const PointerType *PTy = dyn_cast<PointerType>(Ptr);
   if (!PTy) return 0;   // Type isn't a pointer type!
Index: include/llvm/Instructions.h
==================================================================---
include/llvm/Instructions.h	(revision 53716)
+++ include/llvm/Instructions.h	(working copy)
@@ -407,9 +407,6 @@
   /// Null is returned if the indices are invalid for the specified
   /// pointer type.
   ///
-  static const Type *getIndexedType(const Type *Ptr,
-                                    Value* const *Idx, unsigned NumIdx);
-
   template<typename InputIterator>
   static const Type *getIndexedType(const Type *Ptr,
                                     InputIterator IdxBegin, 
@@ -508,6 +505,13 @@
                           typename std::iterator_traits<InputIterator>::
                           iterator_category());
   }  
+
+  static const Type *getIndexedType(const Type *Ptr,
+                                    Value* const *Idx, unsigned NumIdx);
+
+  static const Type *getIndexedType(const Type *Ptr,
+                                    uint64_t const *Idx, unsigned NumIdx);
+
   static const Type *getIndexedType(const Type *Ptr, Value *Idx);
 
   inline op_iterator       idx_begin()       { return op_begin()+1; }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20080723/deee3d06/attachment.sig>

Chris Lattner

2008-Jul-23 17:22 UTC

head link

[LLVMdev] GEP::getIndexValid() with other iterators

On Jul 23, 2008, at 8:43 AM, Matthijs Kooijman wrote:> I did use a template function to prevent duplicating code, but since  
> it is
> only used in the implementation of two getIndexedType() methods, it  
> is in the
> cpp file.
>
> So, is this better?
Looks great to me, please commit!  Thanks Matthijs,

-Chris
>
>
> Gr.
>
> Matthijs
>
> Index: lib/VMCore/Instructions.cpp
> ==================================================================> ---
lib/VMCore/Instructions.cpp	(revision 53716)
> +++ lib/VMCore/Instructions.cpp	(working copy)
> @@ -1071,12 +1071,16 @@
> // getIndexedType - Returns the type of the element that would be  
> loaded with
> // a load instruction with the specified parameters.
> //
> +// The Idxs pointer should point to a continuous piece of memory  
> containing the
> +// indices, either as Value* or uint64_t.
> +//
> // A null type is returned if the indices are invalid for the  
> specified
> // pointer type.
> //
> -const Type* GetElementPtrInst::getIndexedType(const Type *Ptr,
> -                                              Value* const *Idxs,
> -                                              unsigned NumIdx) {
> +template <typename IndexTy>
> +static const Type* getIndexedTypeInternal(const Type *Ptr,
> +                                  IndexTy const *Idxs,
> +                                  unsigned NumIdx) {
>   const PointerType *PTy = dyn_cast<PointerType>(Ptr);
>   if (!PTy) return 0;   // Type isn't a pointer type!
>   const Type *Agg = PTy->getElementType();
> @@ -1089,7 +1093,7 @@
>   for (; CurIdx != NumIdx; ++CurIdx) {
>     const CompositeType *CT = dyn_cast<CompositeType>(Agg);
>     if (!CT || isa<PointerType>(CT)) return 0;
> -    Value *Index = Idxs[CurIdx];
> +    IndexTy Index = Idxs[CurIdx];
>     if (!CT->indexValid(Index)) return 0;
>     Agg = CT->getTypeAtIndex(Index);
>
> @@ -1103,6 +1107,18 @@
>   return CurIdx == NumIdx ? Agg : 0;
> }
>
> +const Type* GetElementPtrInst::getIndexedType(const Type *Ptr,
> +                                              Value* const *Idxs,
> +                                              unsigned NumIdx) {
> +  return getIndexedTypeInternal(Ptr, Idxs, NumIdx);
> +}
> +
> +const Type* GetElementPtrInst::getIndexedType(const Type *Ptr,
> +                                              uint64_t const *Idxs,
> +                                              unsigned NumIdx) {
> +  return getIndexedTypeInternal(Ptr, Idxs, NumIdx);
> +}
> +
> const Type* GetElementPtrInst::getIndexedType(const Type *Ptr, Value  
> *Idx) {
>   const PointerType *PTy = dyn_cast<PointerType>(Ptr);
>   if (!PTy) return 0;   // Type isn't a pointer type!
> Index: include/llvm/Instructions.h
> ==================================================================> ---
include/llvm/Instructions.h	(revision 53716)
> +++ include/llvm/Instructions.h	(working copy)
> @@ -407,9 +407,6 @@
>   /// Null is returned if the indices are invalid for the specified
>   /// pointer type.
>   ///
> -  static const Type *getIndexedType(const Type *Ptr,
> -                                    Value* const *Idx, unsigned  
> NumIdx);
> -
>   template<typename InputIterator>
>   static const Type *getIndexedType(const Type *Ptr,
>                                     InputIterator IdxBegin,
> @@ -508,6 +505,13 @@
>                           typename  
> std::iterator_traits<InputIterator>::
>                           iterator_category());
>   }
> +
> +  static const Type *getIndexedType(const Type *Ptr,
> +                                    Value* const *Idx, unsigned  
> NumIdx);
> +
> +  static const Type *getIndexedType(const Type *Ptr,
> +                                    uint64_t const *Idx, unsigned  
> NumIdx);
> +
>   static const Type *getIndexedType(const Type *Ptr, Value *Idx);
>
>   inline op_iterator       idx_begin()       { return op_begin()+1; }
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

Possibly Parallel Threads

Search for more possibly parallel threads

llvm dev - Jul 2008 - [LLVMdev] GEP::getIndexValid() with other iterators

[LLVMdev] GEP::getIndexValid() with other iterators

[LLVMdev] GEP::getIndexValid() with other iterators

[LLVMdev] GEP::getIndexValid() with other iterators

Possibly Parallel Threads