[Patch] PR rtl-optimization/71150, guard in_class_p check with REG_P

Jiong Wang jiong.wang@foss.arm.com
Tue May 17 11:18:00 GMT 2016


On 17/05/16 11:23, Uros Bizjak wrote:
> On Tue, May 17, 2016 at 12:17 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> Hello!
>>
>>> This bug is introduced by my commit r236181 where the inner rtx of
>>> SUBREG haven't been checked while it should as "in_class_p" only
>>> works with REG, and SUBREG_REG is actually not always REG.  If REG_P
>>> check failed,  then we should fall back to normal code patch. The
>>> following simple testcase for x86 can reproduce this bug.
>>> diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
>>> index 56ab5b4..e4e6c8c 100644
>>> --- a/gcc/lra-constraints.c
>>> +++ b/gcc/lra-constraints.c
>>>   @@ -1317,7 +1317,8 @@ process_addr_reg (rtx *loc, bool check_only_p, rtx_insn **before, rtx_insn **aft
>>>   register, and this normally will be a subreg which should be reloaded
>>>   as a whole.  This is particularly likely to be triggered when
>>>   -fno-split-wide-types specified.  */
>>> -      if (in_class_p (reg, cl, &new_class)
>>> +      if (!REG_P (reg)
>>> +  || in_class_p (reg, cl, &new_class)
>>>    || GET_MODE_SIZE (mode) <= GET_MODE_SIZE (ptr_mode))
>>>         loc = &SUBREG_REG (*loc);
>> Why not check SUBREG_P instead of !REG_P?
> Or, alternatively:
>
> if ((REG_P && !in_class_p (reg, ...))
>      || GET_MODE_SIZE ...)
>
> Which is IMO much more readable.

Thanks for review.

I think your proposed rewrite will be the following,

       if (!(REG_P (reg) && !in_class_p (reg, cl, &new_class))
           || GET_MODE_SIZE (mode) <= GET_MODE_SIZE (ptr_mode))

I feel the original code is easier to understand.
The check logic is composed of three conditions, in a pre-requisite order,
if one condition is true then reload the inner rtx, otherwise reload the 
whole subreg.

       if (!REG_P (reg)
           || in_class_p (reg, cl, &new_class)
           || GET_MODE_SIZE (mode) <= GET_MODE_SIZE (ptr_mode))
        loc = &SUBREG_REG (*loc);






>
> Uros.



More information about the Gcc-patches mailing list