[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.

Hi Eli,

Attached please find the updated patch.
PaddingType currently is only supported for Direct, and this patch will add support of padding for Expand.
If you think I should separate it into two patches, let me know,

Thanks,
Manman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm_ha3.patch
Type: application/octet-stream
Size: 9578 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121029/8809e463/attachment.obj>
-------------- next part --------------

>> 
>> +        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
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits