search for: indexvalid

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 bitwidt...

...ewhere, 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))...

...de being generated in practice? Inserting sign/zero extension instructions actually do tend to increase the generated code pretty dramatically on our target. So I prefer option (1) Does this change have dramatic affect on LLVM IR? PS. Currently (in our local code repository) we have modified the indexValid() method to also allow i16 and so far our test cases are going through. I also tried : float *p1; float *p3; short i; ... p1 = p2 + i; to generate your example: %tmp = sext i16 %idx to i32 %tmp2 = getelementptr float* %P, i32 %tmp On x86; but the sign extension does not get eliminated while lo...

...be zapped by the dag > combiner. Have you actually tried this? Yes, I have and they work just fine for us. I misunderstood your first email, I thought you are pointing at a problem, while you are actually saying that there is no problem. So back to my original question... Would adding i16 to indexValid() do the trick? Or we need to do more? Thanks A.

...Thanks Ali > -----Original Message----- > From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On > Behalf Of Chris Lattner > Sent: Monday, August 18, 2008 4:33 PM > To: LLVM Developers Mailing List > Subject: Re: [LLVMdev] hard values in SequentialType::indexValid () method > > > On Aug 18, 2008, at 4:20 PM, Alireza.Moshtaghi at microchip.com wrote: > > > This method is defined in: > > Lib/VMCore/Type.cpp > > And it makes hard assumption that size of integer is 32 or 64. > > This gives us trouble because we have 16 bit...

...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_ca...

On Aug 18, 2008, at 4:20 PM, Alireza.Moshtaghi at microchip.com wrote: > This method is defined in: > Lib/VMCore/Type.cpp > And it makes hard assumption that size of integer is 32 or 64. > This gives us trouble because we have 16 bit integer. > Is there a reason for this assumption? Or we can just add the 16-bit > integer to it as well? Hi Alireza, The reason for this is that

On Aug 19, 2008, at 6:54 AM, Alireza.Moshtaghi at microchip.com wrote: > What is the reason for getelementptr being tied to only 32 or 64 bit > indexes? As usual, the answer is "that is all anyone has needed so far" :) > Anyways, this is getting in our way. What is your suggestion? Two options: 1) change LLVM IR to allow it, or 2) just insert sign/ zero extension

On Aug 19, 2008, at 4:14 PM, Alireza.Moshtaghi at microchip.com wrote: >> Two options: 1) change LLVM IR to allow it, or 2) just insert sign/ >> zero extension instructions. >> >> Does this actually affect the code being generated in practice? > > Inserting sign/zero extension instructions actually do tend to > increase > the generated code pretty

This method is defined in: Lib/VMCore/Type.cpp And it makes hard assumption that size of integer is 32 or 64. This gives us trouble because we have 16 bit integer. Is there a reason for this assumption? Or we can just add the 16-bit integer to it as well? Thanks Alireza Moshtaghi Senior Software Engineer Development Systems, Microchip Technology

...> > 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 Const...

...)? 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() (...

...e 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....

...gg = 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, +...

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

On Jul 15, 2008, at 3:58 AM, Matthijs Kooijman wrote: > 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

...orTyID, ///< 14: SIMD 'packed' format, or other vector type + UnionTyID, ///< 15: Unions Please put this up next to Struct for simplicity, the numbering here doesn't need to be stable. The numbering in llvm-c/Core.h does need to be stable though. +bool UnionType::indexValid(const Value *V) const { + // Union indexes require 32-bit integer constants. + if (V->getType() == Type::getInt32Ty(V->getContext())) Please use V->getType()->isInteger(32) which is probably new since you started your patch. +UnionType::UnionType(LLVMContext &C, const std::vect...

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

This patch adds a UnionType to DerivedTypes.h. It also adds code to the bitcode reader / writer and the assembly parser for the new type, as well as a tiny .ll test file in test/Assembler. It does not contain any code related to code generation or type layout - I wanted to see if this much was acceptable before I proceeded any further. Unlike my previous patch, in which the Union type was

...value. If you wanted to tackle this, it should be > straight-forward and would be a worthwhile addition to the LLVM > optimizer. In SCCPSolver::visitGetElementPtrInst, it would fall through to the final markConstant call at the end of the function. Someone should be doing a check with indexValid to see if instead it should be marking as undefined. I can try to produce such a patch. >> I thought this might be because the optimizer is being conservative >> about correctness, but that the lattice must still show it as >> underdefined -- but obviously as I haven't looke...