[PATCH] D62152: [ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation

[John Brawn via Phabricator via cfe-commits]

john.brawn added a comment.

In D62152#1508979 <https://reviews.llvm.org/D62152#1508979>, @efriedma wrote:

> Please verify my understanding of the following is correct:
>
> 1. getTypeUnadjustedAlign() is currently only used to compute calling convention alignment for ARM and AArch64.


Yes, the only places it's called are AArch64ABIInfo::classifyArgumentType and ARMABIInfo::classifyArgumentType.

> 2. Without this patch, we use the unadjusted alignment to pass arguments, but the adjusted alignment to compute the alignment for the corresponding va_arg calls.

Yes.

> 3. Without this patch, the unadjusted alignment for non-struct types is actually adjusted based on attributes on typedefs.

Only in va_arg. For example if you look at

  typedef int aligned_int __attribute__((aligned(8)));
  aligned_int called_fn(int a1, aligned_int a2) {
    return a2;
  }
  aligned_int calling_fn() {
    return called_fn(1,2);
  }

a2 is passed in r1 by calling_fn, and read from it in called_fn (both before and after this patch). However for

  typedef int aligned_int __attribute__((aligned(8)));
  aligned_int called_fn(int a1, ...) {
    va_list ap;
    va_start(ap, a1);
    aligned_int a2 = va_arg(ap, aligned_int);
    va_end(ap);
    return a2;
  }
  aligned_int calling_fn() {
    return called_fn(1,2);
  }

a2 is still passed in r1 by calling_fn, but the current clang behaviour in called_fn is that it pushes r1-r3 then loads the next 8-byte-aligned value, which will be the pushed r2 value.  This patch fixes this so that it doesn't 8-byte align the load address, and so gets the pushed r1.

> I'm not confident about changing the calling convention again at this point for non-struct types.  I guess it's obscure enough that changing it is probably okay.  But would you mind splitting it into a separate followup, with a better description of the impact?

This patch only changes the va_arg behaviour, and this whole thing with unadjusted alignment is from the published pcs (https://developer.arm.com/docs/ihi0042/latest/procedure-call-standard-for-the-arm-architecture-abi-2019q1-documentation and https://developer.arm.com/docs/ihi0055/latest/procedure-call-standard-for-the-arm-64-bit-architecture, though it's called "natural alignment" there).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62152/new/

https://reviews.llvm.org/D62152