diff mbox series

Tighten use of HARD_FRAME_POINTER_REGNUM in alias.c (PR 84538)

Message ID 87efl580kb.fsf@linaro.org
State New
Headers show
Series Tighten use of HARD_FRAME_POINTER_REGNUM in alias.c (PR 84538) | expand

Commit Message

Richard Sandiford Feb. 28, 2018, 5:51 p.m. UTC 
RTL code needs to be consistent about whether it uses the stack
pointer, the frame pointer or the argument pointer to access a
given part of the frame.  alias.c used this to divide accesses
into three independent areas.

The problem in the PR is that we did this for HARD_FRAME_POINTER_REGNUM
even when the register wasn't being used as a frame pointer.  We can't
do that because the frame pointer is then just any old allocatable
register and could certainly point to info accessed through the
argument pointer or stack pointer.

In this case the bug triggered wrong code, but diffing the before and after
assembly output for one target per CPU shows that the bug also introduced
an unnecessary scheduling dependence between B and C in things like:

	A: (set hfp (symbol_ref "..."))
	B: ...(mem hfp)...
	C: (clobber (mem sp))

where the hard frame pointer points to something that is obviously
distinct from the frame.  So it looks like this patch is both a
correctness fix and a very minor scheduling improvement.

Tested on aarch64-linux-gnu, and by diffing output as above.  OK to install?

Richard


2018-02-28  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	PR rtl-optimization/84538
	* alias.c (init_alias_target): Add commentary.
	(init_alias_analysis): Only give HARD_FRAME_POINTER_REGNUM
	a unique base value if the frame pointer is not eliminated
	to the stack pointer.

gcc/testsuite/
	PR rtl-optimization/84538
	* gcc.dg/torture/pr84538.c: New test.

Comments

Jeff Law Feb. 28, 2018, 11:06 p.m. UTC | #1 
On 02/28/2018 10:51 AM, Richard Sandiford wrote:
> RTL code needs to be consistent about whether it uses the stack
> pointer, the frame pointer or the argument pointer to access a
> given part of the frame.  alias.c used this to divide accesses
> into three independent areas.
> 
> The problem in the PR is that we did this for HARD_FRAME_POINTER_REGNUM
> even when the register wasn't being used as a frame pointer.  We can't
> do that because the frame pointer is then just any old allocatable
> register and could certainly point to info accessed through the
> argument pointer or stack pointer.
> 
> In this case the bug triggered wrong code, but diffing the before and after
> assembly output for one target per CPU shows that the bug also introduced
> an unnecessary scheduling dependence between B and C in things like:
> 
> 	A: (set hfp (symbol_ref "..."))
> 	B: ...(mem hfp)...
> 	C: (clobber (mem sp))
> 
> where the hard frame pointer points to something that is obviously
> distinct from the frame.  So it looks like this patch is both a
> correctness fix and a very minor scheduling improvement.
> 
> Tested on aarch64-linux-gnu, and by diffing output as above.  OK to install?
> 
> Richard
> 
> 
> 2018-02-28  Richard Sandiford  <richard.sandiford@linaro.org>
> 
> gcc/
> 	PR rtl-optimization/84538
> 	* alias.c (init_alias_target): Add commentary.
> 	(init_alias_analysis): Only give HARD_FRAME_POINTER_REGNUM
> 	a unique base value if the frame pointer is not eliminated
> 	to the stack pointer.
> 
> gcc/testsuite/
> 	PR rtl-optimization/84538
> 	* gcc.dg/torture/pr84538.c: New test.
OK.
jeff
diff mbox series

Patch

Index: gcc/alias.c
===================================================================
--- gcc/alias.c	2018-01-14 08:42:44.497155977 +0000
+++ gcc/alias.c	2018-02-28 17:47:36.486754143 +0000
@@ -3191,12 +3191,21 @@  init_alias_target (void)
 	&& targetm.hard_regno_mode_ok (i, Pmode))
       static_reg_base_value[i] = arg_base_value;
 
+  /* RTL code is required to be consistent about whether it uses the
+     stack pointer, the frame pointer or the argument pointer to
+     access a given area of the frame.  We can therefore use the
+     base address to distinguish between the different areas.  */
   static_reg_base_value[STACK_POINTER_REGNUM]
     = unique_base_value (UNIQUE_BASE_VALUE_SP);
   static_reg_base_value[ARG_POINTER_REGNUM]
     = unique_base_value (UNIQUE_BASE_VALUE_ARGP);
   static_reg_base_value[FRAME_POINTER_REGNUM]
     = unique_base_value (UNIQUE_BASE_VALUE_FP);
+
+  /* The above rules extend post-reload, with eliminations applying
+     consistently to each of the three pointers.  Cope with cases in
+     which the frame pointer is eliminated to the hard frame pointer
+     rather than the stack pointer.  */
   if (!HARD_FRAME_POINTER_IS_FRAME_POINTER)
     static_reg_base_value[HARD_FRAME_POINTER_REGNUM]
       = unique_base_value (UNIQUE_BASE_VALUE_HFP);
@@ -3329,7 +3338,14 @@  init_alias_analysis (void)
 
       /* Initialize the alias information for this pass.  */
       for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
-	if (static_reg_base_value[i])
+	if (static_reg_base_value[i]
+	    /* Don't treat the hard frame pointer as special if we
+	       eliminated the frame pointer to the stack pointer instead.  */
+	    && !(i == HARD_FRAME_POINTER_REGNUM
+		 && reload_completed
+		 && !frame_pointer_needed
+		 && targetm.can_eliminate (FRAME_POINTER_REGNUM,
+					   STACK_POINTER_REGNUM)))
 	  {
 	    new_reg_base_value[i] = static_reg_base_value[i];
 	    bitmap_set_bit (reg_seen, i);
Index: gcc/testsuite/gcc.dg/torture/pr84538.c
===================================================================
--- /dev/null	2018-02-26 10:26:41.624847060 +0000
+++ gcc/testsuite/gcc.dg/torture/pr84538.c	2018-02-28 17:47:36.491754008 +0000
@@ -0,0 +1,16 @@ 
+/* { dg-do run } */
+/* { dg-additional-options "-fno-omit-frame-pointer -w" } */
+
+#define SIZE 8
+
+main()
+{
+  int a[SIZE] = {1};
+  int i;
+
+  for (i = 1; i < SIZE; i++)
+    if (a[i] != 0)
+      abort();
+
+  exit (0);
+}