| Message ID | 87efl580kb.fsf@linaro.org |
|---|---|
| State | Accepted |
| Commit | e6eacdc9451bbee54c80d6b85d22ba390fd2c7c4 |
| Headers | show |
| Series | Tighten use of HARD_FRAME_POINTER_REGNUM in alias.c (PR 84538) | expand |
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
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); +}