[llvm-commits] [llvm] r69946 - in /llvm/trunk: docs/LangRef.html lib/Transforms/Scalar/InstructionCombining.cpp lib/VMCore/Type.cpp

[Sanjiv Gupta]

Chris Lattner wrote:
> On Apr 23, 2009, at 7:37 PM, Sanjiv Gupta wrote:
>> Author: sgupta
>> Date: Thu Apr 23 21:37:54 2009
>> New Revision: 69946
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=69946&view=rev
>> Log:
>> Allow i16 type indices to gep.
>
> Hi Sanjiv,
>
> It looks like you've made all integer sized indices valid for array 
> indexes, see this testcase:
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090420/076706.html 
>
>
> I think this is a fine thing to do, but if you're going to do that, 
> shouldn't SequentialType::indexValid return true for any integer type?
>
> -Chris
I think the assembler and dis-assembler never bothered to call 
sequentialType::indexValid. This test case always used to work.
Its only the clang and the opt that check the validity.

But anyways there is a disconnect here between various tools and the 
documentation. I think we should allow all types of integer indices and 
change the documentation and code accordingly. I will do that.

Meanwhile, do you think we might have a similar case for 
structType::indexValid. Adding a test case for that as well will be a 
good idea.

Thanks,
- Sanjiv
>
>>
>>
>> Modified:
>>    llvm/trunk/docs/LangRef.html
>>    llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp
>>    llvm/trunk/lib/VMCore/Type.cpp
>>
>> Modified: llvm/trunk/docs/LangRef.html
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/LangRef.html?rev=69946&r1=69945&r2=69946&view=diff 
>>
>>
>> ============================================================================== 
>>
>> --- llvm/trunk/docs/LangRef.html (original)
>> +++ llvm/trunk/docs/LangRef.html Thu Apr 23 21:37:54 2009
>> @@ -3629,7 +3629,8 @@
>> <p>The type of each index argument depends on the type it is indexing 
>> into.
>> When indexing into a (packed) structure, only <tt>i32</tt> integer
>> <b>constants</b> are allowed.  When indexing into an array, pointer 
>> or vector,
>> -only integers of 32 or 64 bits are allowed (also non-constants). 
>> 32-bit values
>> +only integers of 16, 32 or 64 bits are allowed (also 
>> non-constants).16-bit
>> +values will be sign extended to 32-bits if required, and  32-bit values
>> will be sign extended to 64-bits if required.</p>
>>
>> <p>For example, let's consider a C code fragment and how it gets
>> @@ -3717,6 +3718,8 @@
>>     %vptr = getelementptr {i32, <2 x i8>}* %svptr, i64 0, i32 
>> 1, i32 1
>>     <i>; yields i8*:eptr</i>
>>     %eptr = getelementptr [12 x i8]* %aptr, i64 0, i32 1
>> +    <i>; yields i16*:iptr</i>
>> +    %iptr = getelementptr [10 x i16]* @arr, i16 0, i16 0
>> </pre>
>> </div>
>>
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp?rev=69946&r1=69945&r2=69946&view=diff 
>>
>>
>> ============================================================================== 
>>
>> --- llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp Thu Apr 
>> 23 21:37:54 2009
>> @@ -10695,12 +10695,7 @@
>>   gep_type_iterator GTI = gep_type_begin(GEP);
>>   for (User::op_iterator i = GEP.op_begin() + 1, e = GEP.op_end();
>>        i != e; ++i, ++GTI) {
>> -    // Before trying to eliminate/introduce cast/ext/trunc to make
>> -    // indices as pointer types, make sure that the pointer size
>> -    // makes a valid sequential index.
>> -    const SequentialType *ST = dyn_cast<SequentialType>(*GTI);
>> -    Value *PtrTypeVal = Constant::getNullValue(TD->getIntPtrType());
>> -    if (ST && ST->indexValid(PtrTypeVal)) {
>> +    if (isa<SequentialType>(*GTI)) {
>>       if (CastInst *CI = dyn_cast<CastInst>(*i)) {
>>         if (CI->getOpcode() == Instruction::ZExt ||
>>             CI->getOpcode() == Instruction::SExt) {
>>
>> Modified: llvm/trunk/lib/VMCore/Type.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Type.cpp?rev=69946&r1=69945&r2=69946&view=diff 
>>
>>
>> ============================================================================== 
>>
>> --- llvm/trunk/lib/VMCore/Type.cpp (original)
>> +++ llvm/trunk/lib/VMCore/Type.cpp Thu Apr 23 21:37:54 2009
>> @@ -1411,7 +1411,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 IT->getBitWidth() == 16 || IT->getBitWidth() == 32 ||
>> +           IT->getBitWidth() == 64;
>>   return false;
>> }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>