Matthijs Kooijman
2008-Jul-16 16:17 UTC
[LLVMdev] GEP::getIndexValid() with other iterators
Hi all, currently, GetElementPtrInst has a method getIndexedType, which has a templated variant. You pass in a begin and an end iterator, and it will find the indexed type for those. However, both iterators must iterate over Value*. For some argpromotion code, I would like to pass in iterators that iterate over unsigneds instead of Value*. I currently solve this by transforming my vector<unsigned> into a vector<Value*>, but this is hardly an efficient solution. Currently, however, there is already a templated version of getIndexedType, probably so it can iterate over any pointer. In there, the begin pointer is cast as follows: (const Value*)&*IdxBegin Though this works, this has the nasty side effect of accepting a lot of iterator types that are not really supported. In particular, when I passed in an iterator over unsigned, the compiler would happily cast the resulting unsigned* to a const Value*, resulting in all kinds of badness at runtime (and not even a warning at compile time). However, simply removing this cast seems to work for me, so I suppose there is some compiler out there that will warn about this? Or is the cast really not needed (anymore)? Anyway, when one removes this cast and also makes the other getIndexedType implementation (which is called by this one) a template, passing in an iterator over unsigned should also work. The actual types over which an iterator is allowed should now be limited by the parameters to CompositeType::indexValid() and CompositeType::getTypeAtIndex() (which currently are Value* or unsigned). I've attached a patch which does exactly this. Since my template-fu is not so strong, please review :-) I've also attached another patch to argpromotion, which passes an unsigned iterator to getIndexedType() (look for the comment // This uses the new templated getIndexdType in the code) Gr. Matthijs -------------- next part -------------- A non-text attachment was scrubbed... Name: argpromotion.diff Type: text/x-diff Size: 24647 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080716/b315b259/attachment.diff> -------------- next part -------------- A non-text attachment was scrubbed... Name: getindexedtype.diff Type: text/x-diff Size: 3738 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080716/b315b259/attachment-0001.diff> -------------- 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/20080716/b315b259/attachment.sig>
Matthijs Kooijman
2008-Jul-16 16:58 UTC
[LLVMdev] GEP::getIndexValid() with other iterators
Hi all,
once more with the patch inline for easy review. I did not include the
argpromotion pass here, since it's not the main topic of this post.
Gr.
Matthijs
Index: lib/VMCore/Instructions.cpp
==================================================================---
lib/VMCore/Instructions.cpp (revision 53672)
+++ lib/VMCore/Instructions.cpp (working copy)
@@ -1068,41 +1068,6 @@
init(Ptr, Idx, Name);
}
-// getIndexedType - Returns the type of the element that would be loaded with
-// a load instruction with the specified parameters.
-//
-// 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) {
- const PointerType *PTy = dyn_cast<PointerType>(Ptr);
- if (!PTy) return 0; // Type isn't a pointer type!
- const Type *Agg = PTy->getElementType();
-
- // Handle the special case of the empty set index set...
- if (NumIdx == 0)
- return Agg;
-
- unsigned CurIdx = 1;
- for (; CurIdx != NumIdx; ++CurIdx) {
- const CompositeType *CT = dyn_cast<CompositeType>(Agg);
- if (!CT || isa<PointerType>(CT)) return 0;
- Value *Index = Idxs[CurIdx];
- if (!CT->indexValid(Index)) return 0;
- Agg = CT->getTypeAtIndex(Index);
-
- // If the new type forwards to another type, then it is in the middle
- // of being refined to another type (and hence, may have dropped all
- // references to what it was using before). So, use the new forwarded
- // type.
- if (const Type *Ty = Agg->getForwardedType())
- Agg = Ty;
- }
- return CurIdx == NumIdx ? Agg : 0;
-}
-
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 53672)
+++ include/llvm/Instructions.h (working copy)
@@ -407,9 +407,35 @@
/// Null is returned if the indices are invalid for the specified
/// pointer type.
///
+ template <typename IndexTy>
static const Type *getIndexedType(const Type *Ptr,
- Value* const *Idx, unsigned NumIdx);
+ 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();
+ // Handle the special case of the empty set index set...
+ if (NumIdx == 0)
+ return Agg;
+
+ unsigned CurIdx = 1;
+ for (; CurIdx != NumIdx; ++CurIdx) {
+ const CompositeType *CT = dyn_cast<CompositeType>(Agg);
+ if (!CT || isa<PointerType>(CT)) return 0;
+ IndexTy Index = Idxs[CurIdx];
+ if (!CT->indexValid(Index)) return 0;
+ Agg = CT->getTypeAtIndex(Index);
+
+ // If the new type forwards to another type, then it is in the middle
+ // of being refined to another type (and hence, may have dropped all
+ // references to what it was using before). So, use the new forwarded
+ // type.
+ if (const Type *Ty = Agg->getForwardedType())
+ Agg = Ty;
+ }
+ return CurIdx == NumIdx ? Agg : 0;
+ }
+
template<typename InputIterator>
static const Type *getIndexedType(const Type *Ptr,
InputIterator IdxBegin,
@@ -422,7 +448,7 @@
if (NumIdx 0)
// This requires that the iterator points to contiguous memory.
- return getIndexedType(Ptr, (Value *const *)&*IdxBegin, NumIdx);
+ return getIndexedType(Ptr, &*IdxBegin, NumIdx);
else
return getIndexedType(Ptr, (Value *const*)0, NumIdx);
}
-------------- 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/20080716/0b844a2d/attachment.sig>
Matthijs Kooijman
2008-Jul-21 15:22 UTC
[LLVMdev] GEP::getIndexValid() with other iterators
Hi all, no comments on this patch? I haven't seen any problems so far, so I'll be committing this tomorrow. Gr. Matthijs -------------- 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/20080721/68964912/attachment.sig>
On Jul 16, 2008, at 9:58 AM, Matthijs Kooijman wrote:> Hi all, > > once more with the patch inline for easy review. I did not include the > argpromotion pass here, since it's not the main topic of this post.Hi Matthijs, 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? -Chris