[PATCH] Correctly align arguments after a byval struct is passed on the stack

[Oliver Stannard]

Indeed, that stack space is still being allocated, just at a later time.
Previously, it was

allocated in code called from the TableGen'd calling convention code, which
resulted

in the arg reg save area being included in the calculation of alignment for
later arguments,

which is incorrect. I have modified it so that this space is allocated in

ARMTargetLowering::LowerFormalArguments, which prevents this problem.

 

Also, this area was previously allocated above the stack pointer, in the
area which

should be owned by the callee, and an extra stack pointer adjustment was
inserted by

the frame lowering code to make space. Now it is allocated below the stack
pointer,

and included in the calculation of the stack frame size, which conceptually
makes more

sense (though it makes no difference to the generated code).

 

Oliver

 

From: Manman Ren [mailto:manman.ren at gmail.com] 
Sent: 04 March 2014 18:05
To: reviews+D2933+public+f1a32e707ae60633 at llvm-reviews.chandlerc.com
Cc: renato.golin at linaro.org; Oliver Stannard; Tim Northover;
llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Correctly align arguments after a byval struct is
passed on the stack

 

 

 

On Tue, Mar 4, 2014 at 3:36 AM, Oliver Stannard <oliver.stannard at arm.com>
wrote:

Hi rengolin,

This patch fixes a bug that would cause an 8-byte-aligned function argument
to be consistently misaligned in some situations involving byval arguments.
The following function signature is one of these cases:

%struct12bytes = type { i32, i32, i32 }
define void @foo(i32 %a, %struct12bytes* byval %b, i64 %c) {

When arguments are initially been allocated, %b has 12 bytes reserved on the
stack, but this is later adjusted to use registers r1..r3 instead of the
stack. However, the 12 bytes are still used when calculating the alignment
of %c, causing %c to be allocated 4 bytes above the top of %b, at stack
offset 0x10. The stack pointer is moved down 12 bytes at the beginning of
the function  to make space for the part of %b which is in registers (all of
it in this example), plus another 4 bytes to maintain 8-byte SP alignment.
However, the 4 bytes of alignment gap between %b and %c are still present,
resulting in %c being loaded from SP+0x14, which is not 8-byte aligned.

The solution to this is to not allocate stack space for the part of a byval
which will be passed in registers.

 

Maybe I didn't get the whole picture, but I think we need to allocate stack
space  for

the part of a byval that will be passed in registers, so the part passed in
registers

and the part passed on stack will be laid out contiguously on stack.

 

Thanks,

Manman

 

This requires some extra work to
calculate the location of byval arguments on the stack, especially in the
case of multiple small byvals in registers, as previously the stack
locations were used to record the location of a byval after storing to the
stack.

This bug can be triggered from C, using a similar test case, but argument b
must be larger than 64 bytes to cause clang to emit a byval argument.

I have put this patch through a night of random PCS testing with no
failures.

http://llvm-reviews.chandlerc.com/D2933

Files:
  include/llvm/CodeGen/CallingConvLower.h
  lib/Target/ARM/ARMFrameLowering.cpp
  lib/Target/ARM/ARMISelLowering.cpp
  lib/Target/ARM/ARMISelLowering.h
  lib/Target/ARM/Thumb1FrameLowering.cpp
  test/CodeGen/ARM/2013-04-05-Small-ByVal-Structs-PR15293.ll
  test/CodeGen/ARM/2013-05-02-AAPCS-ByVal-Structs-C4-C5-VFP.ll
  test/CodeGen/ARM/2014-02-21-byval-reg-split-alignment.ll

_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140305/467a1f35/attachment.html>