This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition


PING ?
Cheers,
Felix


On Wed, Sep 24, 2014 at 8:07 PM, Felix Yang <fei.yang0953@gmail.com> wrote:
> Hi Jeff,
>
>     Thanks for the comments. I updated the patch adding some enhancements.
>     Bootstrapped on x86_64-suse-linux. Please apply this patch if OK for trunk.
>
>     Three points:
>     1. For multiple-set register, it is not qualified to have a equiv
> note once it is marked by no_equiv. The patch is updated with
>        this consideration.
>     2. For the rtx_insn_list new interface, I noticed that the old
> style XEXP accessor macros is still used in function no_equiv.
>        And I choose to the old style macros with this patch and should
> come up with another patch to fix this issue, OK?
>     3. For the conditions that an insn on the init_insns list which
> did not have a note, I reconsider this and find that this can
>        never happens. So I replaced the check with a gcc assertion.
>
>
> Index: gcc/ChangeLog
> ===================================================================
> --- gcc/ChangeLog    (revision 215550)
> +++ gcc/ChangeLog    (working copy)
> @@ -1,3 +1,11 @@
> +2014-09-24  Felix Yang  <felix.yang@huawei.com>
> +
> +    * ira.c (struct equivalence): Add no_equiv member.
> +    (no_equiv): Set no_equiv of struct equivalence if register is marked
> +    as having no known equivalence.
> +    (update_equiv_regs): Check all definitions for a multiple-set
> +    register to make sure that the RHS have the same value.
> +
>  2014-09-24  Jakub Jelinek  <jakub@redhat.com>
>
>      PR sanitizer/63316
> Index: gcc/ira.c
> ===================================================================
> --- gcc/ira.c    (revision 215550)
> +++ gcc/ira.c    (working copy)
> @@ -2900,6 +2900,8 @@ struct equivalence
>    /* Set when an attempt should be made to replace a register
>       with the associated src_p entry.  */
>    char replace;
> +  /* Set if this register has no known equivalence.  */
> +  char no_equiv;
>  };
>
>  /* reg_equiv[N] (where N is a pseudo reg number) is the equivalence
> @@ -3247,6 +3249,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
>    if (!REG_P (reg))
>      return;
>    regno = REGNO (reg);
> +  reg_equiv[regno].no_equiv = 1;
>    list = reg_equiv[regno].init_insns;
>    if (list == const0_rtx)
>      return;
> @@ -3258,7 +3261,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
>      return;
>    ira_reg_equiv[regno].defined_p = false;
>    ira_reg_equiv[regno].init_insns = NULL;
> -  for (; list; list =  XEXP (list, 1))
> +  for (; list; list = XEXP (list, 1))
>      {
>        rtx insn = XEXP (list, 0);
>        remove_note (insn, find_reg_note (insn, REG_EQUIV, NULL_RTX));
> @@ -3373,7 +3376,7 @@ update_equiv_regs (void)
>
>        /* If this insn contains more (or less) than a single SET,
>           only mark all destinations as having no known equivalence.  */
> -      if (set == 0)
> +      if (set == NULL_RTX)
>          {
>            note_stores (PATTERN (insn), no_equiv, NULL);
>            continue;
> @@ -3467,16 +3470,48 @@ update_equiv_regs (void)
>        if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
>          note = NULL_RTX;
>
> -      if (DF_REG_DEF_COUNT (regno) != 1
> -          && (! note
> +      if (DF_REG_DEF_COUNT (regno) != 1)
> +        {
> +          rtx list;
> +          bool equal_p = true;
> +
> +              /* Check if it is possible that this multiple-set register has
> +         a known equivalence.  */
> +          if (reg_equiv[regno].no_equiv)
> +        continue;
> +
> +          if (! note
>            || rtx_varies_p (XEXP (note, 0), 0)
>            || (reg_equiv[regno].replacement
>                && ! rtx_equal_p (XEXP (note, 0),
> -                    reg_equiv[regno].replacement))))
> -        {
> -          no_equiv (dest, set, NULL);
> -          continue;
> +                    reg_equiv[regno].replacement)))
> +        {
> +          no_equiv (dest, set, NULL);
> +          continue;
> +        }
> +
> +          list = reg_equiv[regno].init_insns;
> +          for (; list; list = XEXP (list, 1))
> +        {
> +          rtx note_tmp, insn_tmp;
> +
> +          insn_tmp = XEXP (list, 0);
> +          note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
> +          gcc_assert (note_tmp);
> +          if (! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
> +            {
> +              equal_p = false;
> +              break;
> +            }
> +        }
> +
> +          if (! equal_p)
> +        {
> +          no_equiv (dest, set, NULL);
> +          continue;
> +        }
>          }
> +
>        /* Record this insn as initializing this register.  */
>        reg_equiv[regno].init_insns
>          = gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv[regno].init_insns);
> @@ -3505,10 +3540,9 @@ update_equiv_regs (void)
>           a register used only in one basic block from a MEM.  If so, and the
>           MEM remains unchanged for the life of the register, add a REG_EQUIV
>           note.  */
> -
>        note = find_reg_note (insn, REG_EQUIV, NULL_RTX);
>
> -      if (note == 0 && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS
> +      if (note == NULL_RTX && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS
>            && MEM_P (SET_SRC (set))
>            && validate_equiv_mem (insn, dest, SET_SRC (set)))
>          note = set_unique_reg_note (insn, REG_EQUIV, copy_rtx (SET_SRC (set)));
> Cheers,
> Felix
>
>
> On Wed, Sep 24, 2014 at 1:49 AM, Jeff Law <law@redhat.com> wrote:
>> On 09/23/14 04:51, Felix Yang wrote:
>>>
>>> Hi,
>>>
>>>      Ignore the previous message.
>>>      Attached please find the updated patch.
>>>      Bootstrapped on x86_64-suse-linux. Please apply this patch if OK for
>>> trunk.
>>>
>>> Index: gcc/ChangeLog
>>> ===================================================================
>>> --- gcc/ChangeLog    (revision 215500)
>>> +++ gcc/ChangeLog    (working copy)
>>> @@ -1,3 +1,8 @@
>>> +2014-09-23  Felix Yang  <felix.yang@huawei.com>
>>> +
>>> +    * ira.c (update_equiv_regs): Check all definitions for a multiple-set
>>> +    register to make sure that the RHS have the same value.
>>> +
>>>   2014-09-23  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>
>>>       * cfgcleanup.c (try_optimize_cfg): Do not remove label
>>> Index: gcc/ira.c
>>> ===================================================================
>>> --- gcc/ira.c    (revision 215500)
>>> +++ gcc/ira.c    (working copy)
>>> @@ -3467,16 +3467,43 @@ update_equiv_regs (void)
>>>         if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
>>>           note = NULL_RTX;
>>>
>>> -      if (DF_REG_DEF_COUNT (regno) != 1
>>> -          && (! note
>>> +      if (DF_REG_DEF_COUNT (regno) != 1)
>>> +        {
>>> +          rtx list;
>>
>> This should probably be "rtx_insn_list list".
>>
>>> +          list = reg_equiv[regno].init_insns;
>>> +          for (; list; list = XEXP (list, 1))
>>
>> Please use the next/insn member functions of the rtx_insn_list rather than
>> the old style XEXP accessor macros.
>>
>>
>>> +        {
>>> +          rtx note_tmp, insn_tmp;
>>> +          insn_tmp = XEXP (list, 0);
>>> +          note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
>>> +
>>> +          if (note_tmp == 0
>>> +              || ! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
>>
>> Under what conditions did you find an insn on this list that did not have a
>> note?  There's a deeper question I'm getting to, but let's start here.
>>
>> Rather than use "== 0", if you have an RTX use "== NULL_RTX".  That applies
>> to the note_tmp == 0 test above.
>>
>>
>> Can you make the edits noted above & answer the question WRT insns on the
>> reg_equiv[].init_insns list without notes and repost?
>>
>> Thanks,
>> Jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]