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

[Eli Friedman]

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.

-Eli