Re: [PATCH 10/11] Fix LRA to handle multi-word eliminable registers

[Dimitar Dimitrov <dimitar at dinux dot eu>]

On петък, 22 юни 2018 г. 10:37:10 EEST Jeff Law wrote:
> On 06/21/2018 10:01 PM, Dimitar Dimitrov wrote:
> > On четвъртък, 21 юни 2018 г. 17:03:55 EEST Jeff Law wrote:
> >> On 06/21/2018 11:44 AM, Vladimir Makarov wrote:
> >>> On 06/13/2018 02:58 PM, Dimitar Dimitrov wrote:
> >>>> From: Dimitar Dimitrov <dddimitrov@mm-sol.com>
> >>>> 
> >>>> For some targets, Pmode != UNITS_PER_WORD. Take this into account
> >>>> when marking hard registers as being used.
> >>>> 
> >>>> I tested C and C++ testsuits for x86_64 with and without this
> >>>> patch. There was no regression, i.e. gcc.sum and g++.sum matched
> >>>> exactly.
> >>>> 
> >>>> gcc/ChangeLog:
> >>>> 
> >>>> 2018-06-13  Dimitar Dimitrov  <dimitar@dinux.eu>
> >>>> 
> >>>>     * lra-eliminations.c (set_ptr_hard_reg_bits): New function.
> >>>>     (update_reg_eliminate): Mark all spanning hw registers.
> >>>> 
> >>>> gcc/testsuite/ChangeLog:
> >>>> 
> >>>> 2018-06-13  Dimitar Dimitrov  <dimitar@dinux.eu>
> >>>> 
> >>>>     * gcc.target/pru/lra-framepointer-fragmentation-1.c: New test.
> >>>>     * gcc.target/pru/lra-framepointer-fragmentation-2.c: New test.
> >>>> 
> >>>> Cc: Vladimir Makarov <vmakarov@redhat.com>
> >>>> Cc: Peter Bergner <bergner@vnet.ibm.com>
> >>>> Cc: Kenneth Zadeck <zadeck@naturalbridge.com>
> >>>> Cc: Seongbae Park <seongbae.park@gmail.com>
> >>>> Signed-off-by: Dimitar Dimitrov <dddimitrov@mm-sol.com>
> >>>> ---
> >>>> 
> >>>>   gcc/lra-eliminations.c                             | 14 ++++-
> >>>>   .../pru/lra-framepointer-fragmentation-1.c         | 33 ++++++++++++
> >>>>   .../pru/lra-framepointer-fragmentation-2.c         | 61
> >>>> 
> >>>> ++++++++++++++++++++++
> >>>> 
> >>>>   3 files changed, 106 insertions(+), 2 deletions(-)
> >>>>   create mode 100644
> >>>> 
> >>>> gcc/testsuite/gcc.target/pru/lra-framepointer-fragmentation-1.c
> >>>> 
> >>>>   create mode 100644
> >>>> 
> >>>> gcc/testsuite/gcc.target/pru/lra-framepointer-fragmentation-2.c
> >>>> 
> >>>> diff --git a/gcc/lra-eliminations.c b/gcc/lra-eliminations.c
> >>>> index 21d8d5f8018..566cc2c8248 100644
> >>>> --- a/gcc/lra-eliminations.c
> >>>> +++ b/gcc/lra-eliminations.c
> >>>> @@ -1180,6 +1180,16 @@ spill_pseudos (HARD_REG_SET set)
> >>>> 
> >>>>     bitmap_clear (&to_process);
> >>>>   
> >>>>   }
> >>>>   +static void set_ptr_hard_reg_bits (HARD_REG_SET *hard_reg_set, int
> >>>>   r)
> >>>> 
> >>>> +{
> >>>> +  int w;
> >>>> +
> >>>> +  for (w = 0; w < GET_MODE_SIZE (Pmode); w += UNITS_PER_WORD, r++)
> >>>> +    {
> >>>> +      SET_HARD_REG_BIT (*hard_reg_set, r);
> >>>> +    }
> >>>> +}
> >>>> +
> >>> 
> >>> The patch itself is ok but for uniformity I'd use
> >>> 
> >>>     for (int i = hard_regno_nregs (r, Pmode) - 1; i >= 0; i--)
> >>>     
> >>>       SET_HARD_REG_BIT (*hard_reg_set, r + i);
> >> 
> >> I'm a bit surprised we don't already have a utility function to do this.
> >> Hmmm
> >> 
> >> add_to_hard_reg_set (hard_reg_set, Pmode, r)
> >> 
> >> So instead LRA ought to be using that function in the places where calls
> >> to set_ptr_hard_reg_bits were introduced.
> >> 
> >> Dimitar, can you verify that change works?
> > 
> > Thank you. I'll test it and will update the patch.
> 
> And go ahead and either break out the two new tests.  I suspect we'll
> want to install the LRA patch immediately since it's an independent bugfix.
I verified that add_to_hard_reg_set() fixes the failing tests for PRU.

> 
> > The SET_HARD_REG_BIT call in check_pseudos_live_through_calls also seems
> > suspicous to me. I'll try to come up with a regression test case to
> > justify
> > its upgrade to add_to_hard_reg_set().
> 
> If you can construct a test, great, but from my reading it's clearly
> wrong as well and we ought to fix it too.
I'll send a standalone LRA fix, without PRU tests.

> 
> Jeff
> 
> ps.  Do you have a copyright assignment on file with the FSF for GCC work?
Yes. FSF has my copyright assignment since November 2016 for GCC, Binutils and 
GDB.

Regards,
Dimitar