On Tue, May 15, 2018 at 08:11:21AM -0500, Wilco Dijkstra wrote:
>
> ping
>
>
> From: Wilco Dijkstra
> Sent: 25 October 2017 16:29
> To: GCC Patches
> Cc: nd
> Subject: [PATCH][AArch64] Simplify frame pointer logic
>
>
> Simplify frame pointer logic based on review comments here
> (https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01727.html).
>
> This patch incrementally adds to these frame pointer cleanup patches:
> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00377.html
> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00381.html
>
> Add aarch64_needs_frame_chain to decide when to emit the frame
> chain using clearer logic. Introduce aarch64_use_frame_pointer
> which contains the value of -fno-omit-frame-pointer
> (flag_omit_frame_pointer is set to a magic value so that the mid-end
> won't force the frame pointer in all cases, and leaf frame pointer
> emission can't be supported).
>
> OK for commit?
>
> +/* Determine whether a frame chain needs to be generated. */
> +static bool
> +aarch64_needs_frame_chain (void)
> +{
> + /* Force a frame chain for EH returns so the return address is at FP+8. */
> + if (frame_pointer_needed || crtl->calls_eh_return)
> + return true;
To match the original logic, I think this needs to not return true, but set
some temporary to true which may be overwritten by...
> +
> + /* A leaf function cannot have calls or write LR. */
> + bool is_leaf = crtl->is_leaf && !df_regs_ever_live_p (LR_REGNUM);
> +
> + /* Don't use a frame chain in leaf functions if leaf frame pointers
> + are disabled. */
> + if (flag_omit_leaf_frame_pointer && is_leaf)
> + return false;
This.
> +
> + return aarch64_use_frame_pointer;
> +}
> +
I say that because here
> - /* Force a frame chain for EH returns so the return address is at FP+8. */
> - cfun->machine->frame.emit_frame_chain
> - = frame_pointer_needed || crtl->calls_eh_return;
> -
We fall through to the next check.
> - /* Emit a frame chain if the frame pointer is enabled.
> - If -momit-leaf-frame-pointer is used, do not use a frame chain
> - in leaf functions which do not use LR. */
> - if (flag_omit_frame_pointer == 2
> - && !(flag_omit_leaf_frame_pointer && crtl->is_leaf
> - && !df_regs_ever_live_p (LR_REGNUM)))
> - cfun->machine->frame.emit_frame_chain = true;
> + cfun->machine->frame.emit_frame_chain = aarch64_needs_frame_chain ();
That may well have been a long-standing bug, but I wanted to query it
as you don't mention any bug fixes in the patch cover letter.
Thanks,
James
