[cfe-commits] [PATCH] ARM AAPCS-VFP: fix handling of homogeneous aggregate.

[manman ren]

On Oct 29, 2012, at 2:55 PM, manman ren <mren at apple.com> wrote:

> 
> On Oct 29, 2012, at 2:38 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> 
>> On Mon, Oct 29, 2012 at 10:16 AM, manman ren <mren at apple.com> wrote:
>>> 
>>> If Homogeneous Aggregate can only partially fit into VFP registers, we add padding to make sure
>>> HA will be on stack and later VFP CPRCs will be on stack as well.
>>> 
>>> There is a discussion on whether we should use byval to pass things on stack:
>>> llvm-commits: ABI: how to let the backend know that an aggregate should be allocated on stack
>>> 
>>> For now, the patch implemented what Eli suggested.
>> 
>> +      llvm::Type *PaddingTy = llvm::VectorType::get(
>> +          llvm::Type::getFloatTy(getVMContext()), NumVFPs - PreAllocation);
>> 
>> Using illegal vector types here isn't a good idea, even if it may work
>> for the moment.
> 
> Hi Eli,
> 
> Thanks for reviewing.
> Do you have any suggestion on what type to use as padding here?
> 
>> 
>> +      it->info = ABIArgInfo::getDirect(0, 0, PaddingTy);
>> 
>> getExpandWithPadding would be a bit more clear here.
> Will do.

Aha, found getExpandWithPadding after "make update".
Modified the patch to use getExpandWithPadding.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm_ha4.patch
Type: application/octet-stream
Size: 7494 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121030/6cf45dfb/attachment.obj>
-------------- next part --------------

Thanks,
Manman

>> 
>> +        AllocatedVFP = llvm::RoundUpToAlignment(AllocatedVFP,
>> +                       getContext().getTypeSize(Base) == 64 ? 2 : 4);
>> +        AllocatedVFP += Members * (getContext().getTypeSize(Base) == 64 ? 2:4);
>> 
>> Use a separate variable for "getContext().getTypeSize(Base) == 64 ?
>> 2:4".  Also, I'm not sure we can actually assume all vectors are
>> either 32-bit or 64-bit.
> Will use a separate variable. 
> "Base" here is from isHomogeneousAggregate:
>    // Homogeneous aggregates for AAPCS-VFP must have base types of float,
>    // double, or 64-bit or 128-bit vectors.
> 
>> 
>> +      else {
>> +        AllocatedVFP = llvm::RoundUpToAlignment(AllocatedVFP, 2);
>> +        AllocatedVFP += Members * 2; // Base type is double.
>> +      }
>> 
>> Please assert that the base type is double or long double.
> Will do.
> 
> Thanks,
> Manman
>> 
>> -Eli
>