[llvm-commits] [PATCH] ARM ABI: handle varargs with vector types.

manman ren mren at apple.com
Mon Oct 15 16:46:23 PDT 2012


The initial patch was updated and separated to two patches (attached).

The first patch will fix passing legal vector types as varargs:
We make sure the vector is correctly aligned before casting it to the vector type.

The second patch will fix passing illegal vector types as varargs:
We will legalize them in clang.

Thanks,
Manman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm-abi-vector1.patch
Type: application/octet-stream
Size: 4654 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121015/0c7b0259/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm-abi-vector-all.patch
Type: application/octet-stream
Size: 13412 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121015/0c7b0259/attachment-0001.obj>
-------------- next part --------------


On Oct 11, 2012, at 3:48 PM, manman ren <mren at apple.com> wrote:

> 
> Thanks for the comments.
> 
> Manman
> 
> On Oct 11, 2012, at 2:17 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> 
>> On Thu, Oct 11, 2012 at 10:20 AM, Manman Ren <mren at apple.com> wrote:
>>> 
>>> We used to generate incorrect results when passing vector types as varargs.
>>> 
>>> This patch is intended to fix the issues:
>>> For illegal vector types, we legalize them in clang.
>>> For varargs, make sure the vector is correctly aligned before casting it to the vector type.
>> 
>> Please separate these out into separate patches.
>> 
>> I'm not sure your solution for illegal vectors is appropriate; if the
>> LLVM calling convention code can't do something sane for <3 x i8> on
>> ARM, it's a bug there.
> The main reason I legalize the vectors in clang is for varargs. Since we expand varargs in clang and the call site is handled in the back end, it is hard to match exactly how illegal vectors are handled in the backend.
> 
>> 
>> -  if (!isAggregateTypeForABI(Ty)) {
>> +  if (!isAggregateTypeForABI(Ty) && !isIllegalVectorType(Ty)) {
>>    // Treat an enum type as its underlying type.
>>    if (const EnumType *EnumTy = Ty->getAs<EnumType>())
>>      Ty = EnumTy->getDecl()->getIntegerType();
>> @@ -2919,6 +2920,27 @@
>>            ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
>>  }
>> 
>> +  // Handle illegal vector types here.
>> +  if (isIllegalVectorType(Ty)) {
>> 
>> Weird code structure; just put the isIllegalVectorType check first.
> Will do.
>> 
>> +bool ARMABIInfo::isIllegalVectorType(QualType Ty) const {
>> +  if (const VectorType *VT = Ty->getAs<VectorType>()) {
>> +    // Check whether VT is legal.
>> +    unsigned NumElements = VT->getNumElements();
>> +    // NumElements should be power of 2.
>> +    if ((NumElements & (NumElements - 1)) != 0)
>> +      return true;
>> +    return false;
>> +  }
>> 
>> I'm not sure this actually catches all cases of illegal vector
>> types... e.g. <4 x i1>.
> Will check.
<4 xi1> is illegal, we treat vector types with size <= 32-bits as illegal.
>> 
>> +  if (Ty->getAs<VectorType>() && (Size == 8 || Size >= 16)) {
>> 
>> Size >= 8?
> Will do.
>> 
>> +    for (unsigned WordIdx = 0; WordIdx < Size / 4; WordIdx++) {
>> +      llvm::Value *Src = Builder.CreateLoad(
>> +        Builder.CreateGEP(AddrCast,
>> +                          llvm::ConstantInt::get(CGF.Int32Ty, WordIdx),
>> +                          "var.src"));
>> +      Builder.CreateStore(Src, Builder.CreateGEP(TmpCast,
>> +        llvm::ConstantInt::get(CGF.Int32Ty, WordIdx), "var.dst"));
>> +    }
>> 
>> Please use memcpy instead of explicitly expanding it.
> Will Check.
Updated to use memcpy.
>> 
>> -Eli
> 



More information about the llvm-commits mailing list