This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][AArch64] Remove aarch64_frame_pointer_required


James Greenhalgh <james.greenhalgh@arm.com> writes:
> On Fri, Aug 04, 2017 at 01:41:22PM +0100, Wilco Dijkstra wrote:
>> To implement -fomit-leaf-frame-pointer, there are 2 places where we need
>> to check whether we have to use a frame chain (since register allocation
>> may allocate LR in a leaf function that omits the frame pointer, but if
>> LR is spilled we must emit a frame chain).  To simplify this do not force
>> frame_pointer_needed via aarch64_frame_pointer_required, but enable the
>> frame chain in aarch64_layout_frame.  Now aarch64_frame_pointer_required
>> can be removed and aarch64_can_eliminate is simplified.
>> 
>> OK for commit?
>
> I've thought about this for a while, I'm still not completely comfortable
> with the idea that we "lie" to the mid-end about the frame-pointer, and
> patch it up in the frame layout code, but I can see how we're moving from
> one lie which adds a ton of complexity, to a different lie which reduces
> complexity.
>
> It all still seems tenuous, but I think I understand the reasoning, and
> we've got a solid 6 months to figure out what breaks.

So this really is a few months later (sorry), but I'm not sure it's safe.
emit_frame_chain was introduced on the basis that:

    The current frame code combines the separate concepts of a frame chain
    (saving old FP,LR in a record and pointing new FP to it) and a frame
    pointer used to access locals.

But there's the third question of whether the frame pointer is available
for general allocation.  By removing frame_pointer_required, we're saying
that the frame pointer is always available for general use.  This can be
forced with a crude test case like:

  void g (void);

  int
  f (void)
  {
    register int x asm ("x29");
    asm volatile ("mov %0, 1" : "=r" (x));
    g ();
    return 1;
  }

which at -O2 (with implicit -fno-omit-frame-pointer) generates:

f:
        stp     x29, x30, [sp, -16]!
        mov     x29, sp
#APP
// 7 "/tmp/foo.c" 1
        mov x29, 1
// 0 "" 2
#NO_APP 
        bl      g
        mov     w0, 1
        ldp     x29, x30, [sp], 16
        ret

So we do initialise the frame pointer, but it's not reliable for
backtracing within g.  The same thing could happen in more realistic
functions with high register pressure.

(The above test would generate an error if frame_pointer_required
returned true.)

If we wanted to continue to use sp-based rather than fp-based
accesses with -fno-omit-frame-pointer where possible, we'd probably
need to do that in target-independent code rather than hide it in the
backend.

Thanks,
Richard

> OK (assuming this has been tested *recently* against aarch64-none-linux-gnu).
>
> Reviewed-By: James Greenhalgh <james.greenhalgh@arm.com>
>
> Thanks,
> James
>
>> 
>> ChangeLog:
>> 2017-08-03  Wilco Dijkstra  <wdijkstr@arm.com>
>> 
>>     gcc/
>> 	* config/aarch64/aarch64.c (aarch64_frame_pointer_required)
>> 	Remove.
>> 	(aarch64_layout_frame): Initialise emit_frame_chain.
>> 	(aarch64_can_eliminate): Remove omit leaf frame pointer code.
>> 	(TARGET_FRAME_POINTER_REQUIRED): Remove define.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]