[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?
2017-10-25 Wilco Dijkstra
* config/aarch64/aarch64.c (aarch64_use_frame_pointer):
Add new boolean.
(aarch64_needs_frame_chain): New function.
(aarch64_parse_override_string): Set aarch64_parse_override_string.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
0c0edc100ee4197d027e2d80da56a12b9129fca4..9abde0d66d52ac4fd17d1dc2193a6bbb5f9c65fc
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -163,6 +163,9 @@ unsigned long aarch64_tune_flags = 0;
/* Global flag for PC relative loads. */
bool aarch64_pcrelative_literal_loads;
+/* Global flag for whether frame pointer is enabled. */
+bool aarch64_use_frame_pointer;
+
/* Support for command line parsing of boolean flags in the tuning
structures. */
struct aarch64_flag_desc
@@ -2871,6 +2874,25 @@ aarch64_output_probe_stack_range (rtx reg1, rtx reg2)
return "";
}
+/* 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;
+
+ /* 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;
+
+ return aarch64_use_frame_pointer;
+}
+
/* Mark the registers that need to be saved by the callee and calculate
the size of the callee-saved registers area and frame record (both FP
and LR may be omitted). */
@@ -2883,17 +2905,7 @@ aarch64_layout_frame (void)
if (reload_completed && cfun->machine->frame.laid_out)
return;
- /* 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;
-
- /* 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 ();
#define SLOT_NOT_REQUIRED (-2)
#define SLOT_REQUIRED (-1)
@@ -9152,12 +9164,12 @@ aarch64_override_options_after_change_1 (struct
gcc_options *opts)
/* PR 70044: We have to be careful about being called multiple times for the
same function. This means all changes should be repeatable. */
- /* If the frame pointer is enabled, set it to a special value that behaves
- similar to frame pointer omission. If we don't do this all leaf functions
- will get a frame pointer even if flag_omit_leaf_frame_pointer is set.
- If flag_omit_frame_pointer has this special value, we must force the
- frame pointer if not in a leaf function. We also need to force it in a
- leaf function if flag_omit_frame_pointer is not set or if LR is used. */
+ /* Set aarch64_use_frame_pointer based on -fno-omit-frame-pointer.
+ Disable the frame pointer flag so the mid-end will not use a frame
+ pointer in leaf functions in order to support -fomit-leaf-frame-pointer.
+ Set x_flag_omit_frame_pointer to the special value 2 to differentiate
+ between -fomit-frame-pointer (1) and -fno-omit-frame-pointer (2). */
+ aarch64_use_frame_pointer = opts->x_flag_omit_frame_pointer != 1;
if (opts->x_flag_omit_frame_pointer == 0)
opts->x_flag_omit_frame_pointer = 2;
Re: [PATCH][AArch64] Simplify frame pointer logic
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?
2017-10-25 Wilco Dijkstra
* config/aarch64/aarch64.c (aarch64_use_frame_pointer):
Add new boolean.
(aarch64_needs_frame_chain): New function.
(aarch64_parse_override_string): Set aarch64_parse_override_string.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
0c0edc100ee4197d027e2d80da56a12b9129fca4..9abde0d66d52ac4fd17d1dc2193a6bbb5f9c65fc
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -163,6 +163,9 @@ unsigned long aarch64_tune_flags = 0;
/* Global flag for PC relative loads. */
bool aarch64_pcrelative_literal_loads;
+/* Global flag for whether frame pointer is enabled. */
+bool aarch64_use_frame_pointer;
+
/* Support for command line parsing of boolean flags in the tuning
structures. */
struct aarch64_flag_desc
@@ -2871,6 +2874,25 @@ aarch64_output_probe_stack_range (rtx reg1, rtx reg2)
return "";
}
+/* 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;
+
+ /* 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;
+
+ return aarch64_use_frame_pointer;
+}
+
/* Mark the registers that need to be saved by the callee and calculate
the size of the callee-saved registers area and frame record (both FP
and LR may be omitted). */
@@ -2883,17 +2905,7 @@ aarch64_layout_frame (void)
if (reload_completed && cfun->machine->frame.laid_out)
return;
- /* 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;
-
- /* 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 ();
#define SLOT_NOT_REQUIRED (-2)
#define SLOT_REQUIRED (-1)
@@ -9152,12 +9164,12 @@ aarch64_override_options_after_change_1 (struct
gcc_options *opts)
/* PR 70044: We have to be careful about being called multiple times for the
same function. This means all changes should be repeatable. */
- /* If the frame pointer is enabled, set it to a special value that behaves
- similar to frame pointer omission. If we don't do this all leaf functions
- will get a frame pointer even if flag_omit_leaf_frame_pointer is set.
- If flag_omit_frame_pointer has this special value, we must force the
- frame pointer if not in a leaf function. We also need to force it in a
- leaf function if flag_omit_frame_pointer is not set or if LR is used. */
+ /* Set aarch64_use_frame_pointer based on -fno-omit-frame-pointer.
+ Disable the frame pointer flag so the mid-end will not use a frame
+ pointer in leaf functions in order to support -fomit-leaf-frame-pointer.
+ Set x_flag_omit_frame_pointer to the special value 2 to differentiate
+ between -fomit-frame-pointer (1) and -fno-omit-frame-pointer (2). */
+ aarch64_use_frame_pointer = opts->x_flag_omit_frame_pointer != 1;
if (opts->x_flag_omit_frame_pointer == 0)
opts->x_flag_omit_frame_pointer = 2;
Re: [PATCH][AArch64] Simplify frame pointer logic
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
Re: [PATCH][AArch64] Simplify frame pointer logic
James Greenhalgh wrote:
> +/* 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...
It's only overwritten if false, once true it cannot ever become false.
> +
> + /* 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.
No, a leaf function with alloca or EH return still must use a frame pointer.
> +
> + 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.
There is no bug here, the code is non-trivial but it does exactly what it says -
forcing the frame chain on when required in a non-leaf or a leaf if leaf frame
pointer
omission is disabled. The point of the new code is to make it a bit simpler to
read.
Wilco
Re: [PATCH][AArch64] Simplify frame pointer logic
On Tue, May 22, 2018 at 10:37:30AM -0500, Wilco Dijkstra wrote:
> James Greenhalgh wrote:
>
> > +/* 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...
>
> It's only overwritten if false, once true it cannot ever become false.
Doh, of course.
OK for trunk.
Thanks,
James
