Hi Evan,> I don't think this is right. According to llvm documentation: > > The index types specified for the 'getelementptr' instruction depend on the > pointer type that is being indexed into. Pointer and array types can use a > 32-bit or 64-bit integer type but the value will always be sign extended to > 64-bits. Structure and packed structure types require i32 constants.That's correct. I'm not saying to allow other integer types for GEP instructions. The first two methods below are methods in StructType and SequentialType. Regardless of any limitations posed by a GEP instruction, I argue that "i8 2" is a perfectly valid index for a struct. The fact that it is not valid for a GEP instruction, should be checked somewhere else (GEPInst constructor or GEPInst::getIndexedValue probably). The last method in the patch is directly related to GEP indices, so asserting only the constness might not be enough, though it would not hurt to make this assert slightly more general (ie, don't check the bitwidth as well). 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/20080710/7ee7033c/attachment.sig>
On Jul 10, 2008, at 12:26 PM, Matthijs Kooijman wrote:> Hi Evan, > > >> I don't think this is right. According to llvm documentation: >> >> The index types specified for the 'getelementptr' instruction >> depend on the >> pointer type that is being indexed into. Pointer and array types >> can use a >> 32-bit or 64-bit integer type but the value will always be sign >> extended to >> 64-bits. Structure and packed structure types require i32 constants. > That's correct. I'm not saying to allow other integer types for GEP > instructions. The first two methods below are methods in StructType > and > SequentialType. Regardless of any limitations posed by a GEP > instruction, I > argue that "i8 2" is a perfectly valid index for a struct. The fact > that it is > not valid for a GEP instruction, should be checked somewhere else > (GEPInst > constructor or GEPInst::getIndexedValue probably).That's what the documentation is saying though. Struct and packed structure types require i32 constants. Perhaps I am misunderstanding something? Evan> > > The last method in the patch is directly related to GEP indices, so > asserting > only the constness might not be enough, though it would not hurt to > make this > assert slightly more general (ie, don't check the bitwidth as well). > > Gr. > > Matthijs > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Jul 10, 2008, at 12:26 PM, Matthijs Kooijman wrote:> Regardless of any limitations posed by a GEP instruction, I > argue that "i8 2" is a perfectly valid index for a struct.Why? What value does that provide? Struct indices are not allowed to be variable, so picking any width constant (that isn't too small) is fine. What problem are you trying to solve here? -Chris
Hi Chris & Evan, On Sun, Jul 13, 2008 at 03:34:03PM -0700, Chris Lattner wrote:> Why? What value does that provide? Struct indices are not allowed to > be variable, so picking any width constant (that isn't too small) is > fine. What problem are you trying to solve here?The current code only allows 32 bit constants. My patch allows any width constant to be used, but it seems Evan doesn't agree with that. On Fri, Jul 11, 2008 at 10:56:16AM -0700, Evan Cheng wrote:> >> I don't think this is right. According to llvm documentation: > >> > >> The index types specified for the 'getelementptr' instruction depend on > >> the pointer type that is being indexed into. Pointer and array types can > >> use a 32-bit or 64-bit integer type but the value will always be sign > >> extended to 64-bits. Structure and packed structure types require i32 > >> constants. > > That's correct. I'm not saying to allow other integer types for GEP > > instructions. The first two methods below are methods in StructType and > > SequentialType. Regardless of any limitations posed by a GEP instruction, > > I argue that "i8 2" is a perfectly valid index for a struct. The fact > > that it is not valid for a GEP instruction, should be checked somewhere > > else (GEPInst constructor or GEPInst::getIndexedValue probably). > That's what the documentation is saying though. Struct and packed > structure types require i32 constants. Perhaps I am misunderstanding > something?Ah, I did miss the "only 32 bit" limitation for structs, which makes my current argpromotion fix wrong. However, I still think that that is a GEP limitation, that does not belong with StructType. Or is there some other piece of documentation you are referring to? Gr. Matthijs