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

[manman ren]

On Oct 16, 2012, at 12:22 PM, manman ren <mren at apple.com> wrote:

> 
> 
> On Oct 16, 2012, at 12:04 PM, manman ren <mren at apple.com> wrote:
> 
>> 
>> Committed as r166040.
>> Thanks Eli for reviewing.
>> 
>> Manman
>> 
>> On Oct 16, 2012, at 11:53 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> 
>>> On Tue, Oct 16, 2012 at 11:42 AM, Manman Ren <mren at apple.com> wrote:
>>>> 
>>>> On Oct 16, 2012, at 11:32 AM, Eli Friedman wrote:
>>>> 
>>>>> On Tue, Oct 16, 2012 at 11:10 AM, manman ren <mren at apple.com> wrote:
>>>>>> 
>>>>>> On Oct 15, 2012, at 5:59 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>>>>> 
>>>>>>> On Mon, Oct 15, 2012 at 5:38 PM, Manman Ren <mren at apple.com> wrote:
>>>>>>>> 
>>>>>>>> On Oct 15, 2012, at 5:34 PM, Eli Friedman wrote:
>>>>>>>> 
>>>>>>>>> On Mon, Oct 15, 2012 at 5:26 PM, Manman Ren <mren at apple.com> wrote:
>>>>>>>>>> 
>>>>>>>>>> On Oct 15, 2012, at 5:11 PM, Eli Friedman wrote:
>>>>>>>>>> 
>>>>>>>>>>> On Mon, Oct 15, 2012 at 4:46 PM, manman ren <mren at apple.com> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>> 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.
>>>>>>>>>>> 
>>>>>>>>>>> +  uint64_t Size = CGF.getContext().getTypeSize(Ty) / 8;
>>>>>>>>>>> +  uint64_t TyAlign = CGF.getContext().getTypeAlign(Ty) / 8;
>>>>>>>>>>> +
>>>>>>>>>>> +  // The ABI alignment for vectors is 8 for AAPCS and 4 for APCS.
>>>>>>>>>>> +  if (Ty->getAs<VectorType>() && Size >= 8) {
>>>>>>>>>>> +    if (getABIKind() == ARMABIInfo::AAPCS_VFP ||
>>>>>>>>>>> +        getABIKind() == ARMABIInfo::AAPCS)
>>>>>>>>>>> +      TyAlign = 8;
>>>>>>>>>>> +    else
>>>>>>>>>>> +      TyAlign = 4;
>>>>>>>>>>> +  }
>>>>>>>>>>> 
>>>>>>>>>>> We should be using the same rules here we do for argument passing.  In
>>>>>>>>>>> particular, for APCS, the argument-passing type alignment is
>>>>>>>>>>> unconditionally 4.  (This can have effects for structs marked with
>>>>>>>>>>> "__attribute__((aligned(16)))", etc.)
>>>>>>>>>> Is there an interface to query the argument-passing alignment?
>>>>>>>>> 
>>>>>>>>> Nothing cares about it other than the calling convention code, so
>>>>>>>>> there isn't a general API.  It's basically just "match whatever
>>>>>>>>> ARMABIInfo::classifyArgumentType does".
>>>>>>>> 
>>>>>>>> I don't see code in ARMABIInfo::classifyArgumentType that checks the argument-passing alignment.
>>>>>>>> The code in the patch is trying to match what the calling convention does in the backend:
>>>>>>> 
>>>>>>> Yes, it's a bit tricky because of the implicit contract between clang
>>>>>>> and the ARM backend... you basically have to combine the frontend and
>>>>>>> backend rules to figure out how the result is actually represented on
>>>>>>> the stack.  When you do that, it comes out to always 4 for APCS, and I
>>>>>>> think it comes out to min(max(naturalAlign, 4), 8) for AAPCS.
>>>>>> 
>>>>>> I think you are right, the patch is updated, please review again :]
>>>>> 
>>>>> +  if (Ty->getAs<VectorType>()) {
>>>>> 
>>>>> Why restrict the check to vector types?  (This check can have effects
>>>>> for other types, e.g. structs containing vectors.)
>>>> I want to make sure this patch only touches varargs with vector types.
>>>> For other types, we can handle those in a separate patch with additional testing cases.
> 
> Committed the patch for illegal vector types in r166043.
> I will try to remove the check for vector types and add a testing case.
Committed as r166052.

Thanks,
Manman
> 
> Thanks,
> Manman
> 
>>> 
>>> Okay.
>>> 
>>> -Eli
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>