PING ? Cheers, Felix
On Wed, Sep 24, 2014 at 8:07 PM, Felix Yang <fei.yang0...@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.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