Hi all, I'm fiddling around a bit with programmatically created GEP instructions, which is failing when using i64 for any indix, except the first. The reason behind this, is that StructureType::indexValid only accepts Value* that are constant ints of width 32. I can't really see why this limitation is here. SequentialType solves this slightly differently, it allows for both 32 and 64 bits integers. However, AFAICS, any constant integer should do just fine, so I would propose to drop the bitwidth check such that any ConstantInt is valid. This change should also be made to SequentialType, which can be indexed by any integer type as well. I've attached a patch, which makes any integer type valid for indexing an CompositeType. The side effect is now that GEP no longer poses any conditions on it's indices (since it delegated that to indexValid, which is not the right place IMHO). I guess those checks need to explicitely added somewhere in GetElementPtrInst. Gr. Matthijs -------------- next part -------------- A non-text attachment was scrubbed... Name: indexvalid.diff Type: text/x-diff Size: 896 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080707/35e6c5d7/attachment.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/20080707/35e6c5d7/attachment.sig>
Hi all, I just found that TargetData also contains a similar assert, and that I forgot a "return" in my previous patch. Here's an updated patch, which allows to use any integer to index a CompositeType. It seems this enables GEP to work with any integer value as well, I tested indexing a struct with a i16 an i64 (also through llc). This should still be adressed somewhere, unless we want to remove the i32 or i64 constraint from GEP. Gr. Matthijs Index: lib/VMCore/Type.cpp ==================================================================--- lib/VMCore/Type.cpp (revision 53136) +++ lib/VMCore/Type.cpp (working copy) @@ -394,9 +394,8 @@ bool StructType::indexValid(const Value *V) const { // Structure indexes require 32-bit integer constants. - if (V->getType() == Type::Int32Ty) - if (const ConstantInt *CU = dyn_cast<ConstantInt>(V)) - return indexValid(CU->getZExtValue()); + if (const ConstantInt *CU = dyn_cast<ConstantInt>(V)) + return indexValid(CU->getZExtValue()); return false; } @@ -1530,9 +1529,8 @@ } bool SequentialType::indexValid(const Value *V) const { - if (const IntegerType *IT = dyn_cast<IntegerType>(V->getType())) - return IT->getBitWidth() == 32 || IT->getBitWidth() == 64; - return false; + // Any integer value is allowed + return V->getType()->isInteger(); } namespace llvm { Index: lib/Target/TargetData.cpp ==================================================================--- lib/Target/TargetData.cpp (revision 53136) +++ lib/Target/TargetData.cpp (working copy) @@ -553,8 +553,7 @@ TI = gep_type_begin(ptrTy, Indices, Indices+NumIndices); for (unsigned CurIDX = 0; CurIDX != NumIndices; ++CurIDX, ++TI) { if (const StructType *STy = dyn_cast<StructType>(*TI)) { - assert(Indices[CurIDX]->getType() == Type::Int32Ty && - "Illegal struct idx"); + assert(isa<ConstantInt>(Indices[CurIDX]) && "Illegal struct idx"); unsigned FieldNo = cast<ConstantInt>(Indices[CurIDX])->getZExtValue(); // Get structure layout information... -------------- 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/20080707/e91dd8bc/attachment.sig>
Hi all, any comments about this patch? I'd like to get it out of my working copy :-) Gr. Matthijs> Index: lib/VMCore/Type.cpp > ==================================================================> --- lib/VMCore/Type.cpp (revision 53136) > +++ lib/VMCore/Type.cpp (working copy) > @@ -394,9 +394,8 @@ > > bool StructType::indexValid(const Value *V) const { > // Structure indexes require 32-bit integer constants. > - if (V->getType() == Type::Int32Ty) > - if (const ConstantInt *CU = dyn_cast<ConstantInt>(V)) > - return indexValid(CU->getZExtValue()); > + if (const ConstantInt *CU = dyn_cast<ConstantInt>(V)) > + return indexValid(CU->getZExtValue()); > return false; > } > > @@ -1530,9 +1529,8 @@ > } > > bool SequentialType::indexValid(const Value *V) const { > - if (const IntegerType *IT = dyn_cast<IntegerType>(V->getType())) > - return IT->getBitWidth() == 32 || IT->getBitWidth() == 64; > - return false; > + // Any integer value is allowed > + return V->getType()->isInteger(); > } > > namespace llvm { > Index: lib/Target/TargetData.cpp > ==================================================================> --- lib/Target/TargetData.cpp (revision 53136) > +++ lib/Target/TargetData.cpp (working copy) > @@ -553,8 +553,7 @@ > TI = gep_type_begin(ptrTy, Indices, Indices+NumIndices); > for (unsigned CurIDX = 0; CurIDX != NumIndices; ++CurIDX, ++TI) { > if (const StructType *STy = dyn_cast<StructType>(*TI)) { > - assert(Indices[CurIDX]->getType() == Type::Int32Ty && > - "Illegal struct idx"); > + assert(isa<ConstantInt>(Indices[CurIDX]) && "Illegal struct idx"); > unsigned FieldNo = cast<ConstantInt>(Indices[CurIDX])->getZExtValue(); > > // Get structure layout information...-------------- 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/20080710/b7db6765/attachment.sig>