[PATCH] Make Scalar Replacement of Aggregates use the right opcode for converting from vector-of-int to vector-of-pointer

[Benjamin Kramer]

On 21.09.2013, at 15:05, Benjamin Kramer <benny.kra at gmail.com> wrote:

> 
> On 20.09.2013, at 20:55, Tom Roeder <tmroeder at google.com> wrote:
> 
>> Index: lib/Transforms/Scalar/SROA.cpp
>> ===================================================================
>> --- lib/Transforms/Scalar/SROA.cpp	(revision 191100)
>> +++ lib/Transforms/Scalar/SROA.cpp	(working copy)
>> @@ -1460,6 +1460,22 @@
>>     return false;
>>   }
>> 
>> +  if (NewTy->isVectorTy() || OldTy->isVectorTy()) {
>> +    // Since the two vectors were checked above to have the same size in bits,
>> +    // any two vector types can be converted with bitcast, inttoptr, or
>> +    // ptrtotint.
>> +    if (NewTy->isVectorTy() && OldTy->isVectorTy())
>> +      return true;
>> +
>> +    // At this point, exactly one of OldTy and NewTy is a vector type.
>> +    // The conversion between <n x type*> and any other non-vector type is not
>> +    // possible. So, check that the vector type does not have a pointer element
>> +    // type.
> 
> Not possible is a bit hard in wording, it just needs an intermediate bitcast step through the intptr type.

After discussing the issue with Chandler it became clear that it's preferable to be able to SROA those cases too instead of just disabling giving up. I rewrote your patch with support for those cases and committed in r191143. Thanks for the great test case!

- Ben

>> +    VectorType *VecTy = cast<VectorType>(OldTy->isVectorTy() ? OldTy : NewTy);
>> +    if (VecTy->getElementType()->isPointerTy())
>> +      return false;
>> +  }
>> +
>>   return true;
>> }
>> 
>> @@ -1479,9 +1495,16 @@
>>     if (IntegerType *NewITy = dyn_cast<IntegerType>(Ty))
>>       if (NewITy->getBitWidth() > OldITy->getBitWidth())
>>         return IRB.CreateZExt(V, NewITy);
>> -  if (V->getType()->isIntegerTy() && Ty->isPointerTy())
>> +  Type *SrcTy = V->getType();
>> +  Type *DstTy = Ty;
> 
> I think renaming the parameters from the beginning of the function is a good idea, preferably using the same naming scheme (OldTy, NewTy) as canConvertValue. Typing V->getType() all the time is tiresome.
> 
>> +  if (VectorType *SrcVecTy = dyn_cast<VectorType>(SrcTy))
>> +    SrcTy = SrcVecTy->getElementType();
>> +  if (VectorType *DstVecTy = dyn_cast<VectorType>(DstTy))
>> +    DstTy = DstVecTy->getElementType();
> 
> This could use Type->getScalarType() which returns the element type if it's a vector or the type if it's not. It could also be inlined into the checks below.
> 
> - Ben
> 
>> +
>> +  if (SrcTy->isIntegerTy() && DstTy->isPointerTy())
>>     return IRB.CreateIntToPtr(V, Ty);
>> -  if (V->getType()->isPointerTy() && Ty->isIntegerTy())
>> +  if (SrcTy->isPointerTy() && DstTy->isIntegerTy())
>>     return IRB.CreatePtrToInt(V, Ty);
>> 
>>   return IRB.CreateBitCast(V, Ty);
>