[cfe-commits] r167416 - /cfe/trunk/lib/CodeGen/TargetInfo.cpp

[Manman Ren]

On Nov 5, 2012, at 2:58 PM, Eli Friedman <eli.friedman at gmail.com> wrote:

> On Mon, Nov 5, 2012 at 2:42 PM, Manman Ren <mren at apple.com> wrote:
>> Author: mren
>> Date: Mon Nov  5 16:42:46 2012
>> New Revision: 167416
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=167416&view=rev
>> Log:
>> ARM byval: when type alignment is bigger than ABI alignment, we can't guarantee
>> the type alignment of the byval argument. This patch will disable byval in this case,
>> it also increases the size threshold for turning on byval.
>> 
>> A backend fix will be attempted.
>> 
>> rdar://12596507
>> 
>> Modified:
>>   cfe/trunk/lib/CodeGen/TargetInfo.cpp
>> 
>> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=167416&r1=167415&r2=167416&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Mon Nov  5 16:42:46 2012
>> @@ -3220,8 +3220,16 @@
>>  }
>> 
>>  // Support byval for ARM.
>> -  if (getContext().getTypeSizeInChars(Ty) > CharUnits::fromQuantity(64) ||
>> -      getContext().getTypeAlign(Ty) > 64) {
>> +  // The ABI alignment for APCS is 4-byte and for AAPCS at least 4-byte and at most 8-byte.
>> +  // Byval can't handle the case where type alignment is bigger than ABI alignment.
>> +  // We also increase the threshold for byval due to its overhead.
>> +  uint64_t ABIAlign = 4;
>> +  uint64_t TyAlign = getContext().getTypeAlign(Ty) / 8;
>> +  if (getABIKind() == ARMABIInfo::AAPCS_VFP ||
>> +      getABIKind() == ARMABIInfo::AAPCS)
>> +    ABIAlign = std::min(std::max(TyAlign, (uint64_t)4), (uint64_t)8);
>> +  if (getContext().getTypeSizeInChars(Ty) > CharUnits::fromQuantity(64*8) &&
>> +      TyAlign <= ABIAlign) {
>>    return ABIArgInfo::getIndirect(0, /*ByVal=*/true);
>>  }
> 
> ABIArgInfo::getIndirect takes an optional "Realign"; it's possible
> this code should be using it.

Thanks, Eli.
I will try that.

Manman
> 
> -Eli