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

[Jeff Law <law at redhat dot com>]

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.
> 
> 
> 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().
I wouldn't be surprised if there's others, particularly WRT Pmode.
Targets where Pmode != word_mode are relatively rare and those that
exist aren't extensively tested.

Jeff
> 
> Regards,
> Dimitar
>