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.y...@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 <ja...@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 <l...@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.y...@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.enkov...@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: gcc/ChangeLog =================================================================== --- gcc/ChangeLog (revision 215550) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,11 @@ +2014-09-24 Felix Yang <felix.y...@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 <ja...@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)));