On 08/24/2017 12:25 PM, Richard Sandiford wrote:
> Segher Boessenkool <seg...@kernel.crashing.org> writes:
>> On Wed, Aug 23, 2017 at 11:49:03AM +0100, Richard Sandiford wrote:
>>> This patch uses df_read_modify_subreg_p to check whether writing
>>> to a subreg would preserve some of the existing contents.
>>
>> combine does not keep the DF info up-to-date -- but that is no
>> problem here, df_read_modify_subreg_p uses no DF info at all.  Maybe
>> it should not have "df_" in the name?
> 
> Yeah, I guess that's a bit confusing.  I've just posted a patch
> to rename it.
> 
> Here's a version of the patch that applies on top of that one.
> Tested as before.  OK to install?
> 
> Thanks,
> Richard
> 
> 
> 2017-08-24  Richard Sandiford  <richard.sandif...@linaro.org>
>           Alan Hayward  <alan.hayw...@arm.com>
>           David Sherwood  <david.sherw...@arm.com>
> 
> gcc/
>       * caller-save.c (mark_referenced_regs):  Use read_modify_subreg_p.
>       * combine.c (find_single_use_1): Likewise.
>       (expand_field_assignment): Likewise.
>       (move_deaths): Likewise.
>       * lra-constraints.c (simplify_operand_subreg): Likewise.
>       (curr_insn_transform): Likewise.
>       * lra.c (collect_non_operand_hard_regs): Likewise.
>       (add_regs_to_insn_regno_info): Likewise.
>       * rtlanal.c (reg_referenced_p): Likewise.
>       (covers_regno_no_parallel_p): Likewise.
> 


> Index: gcc/combine.c
> ===================================================================
> --- gcc/combine.c     2017-08-24 19:22:26.163269637 +0100
> +++ gcc/combine.c     2017-08-24 19:22:45.218100970 +0100
> @@ -579,10 +579,7 @@ find_single_use_1 (rtx dest, rtx *loc)
>         && !REG_P (SET_DEST (x))
>         && ! (GET_CODE (SET_DEST (x)) == SUBREG
>               && REG_P (SUBREG_REG (SET_DEST (x)))
> -             && (((GET_MODE_SIZE (GET_MODE (SUBREG_REG (SET_DEST (x))))
> -                   + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD)
> -                 == ((GET_MODE_SIZE (GET_MODE (SET_DEST (x)))
> -                      + (UNITS_PER_WORD - 1)) / UNITS_PER_WORD))))
> +             && !read_modify_subreg_p (SET_DEST (x))))
>       break;
Is this correct for a paradoxical subreg?  ISTM the original code was
checking for a subreg that just changes the mode, but not the size
(subreg:SI (reg:SF)) or (subreg:DF (reg:DI)) kinds of things.  It would
reject a paradoxical AFAICT.

As written now I think the condition would be true for a paradoxical.

Similarly for the other two instances in combine.c and the changes in
rtlanal.c.

In some of those cases you might be able to argue that it's the right
way to handle a paradoxical.  I haven't thought a whole lot about that
angle, but mention it as a possible way your change might still be correct.


Jeff

Reply via email to