On Tue, Oct 16, 2012 at 11:10 AM, manman ren <[email protected]> wrote:
>
> On Oct 15, 2012, at 5:59 PM, Eli Friedman <[email protected]> wrote:
>
>> On Mon, Oct 15, 2012 at 5:38 PM, Manman Ren <[email protected]> wrote:
>>>
>>> On Oct 15, 2012, at 5:34 PM, Eli Friedman wrote:
>>>
>>>> On Mon, Oct 15, 2012 at 5:26 PM, Manman Ren <[email protected]> wrote:
>>>>>
>>>>> On Oct 15, 2012, at 5:11 PM, Eli Friedman wrote:
>>>>>
>>>>>> On Mon, Oct 15, 2012 at 4:46 PM, manman ren <[email protected]> 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.)

+  if (Ty->getAs<VectorType>() &&

Same here.

-Eli
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to