Thanks for the explaination. I have changed the loop_depth into a short interger hoping that we can save some memory :-) Attached please find the updated patch. Bootstrapped and reg-tested on x86_64-suse-linux. Please do a final revew once the assignment is ready.
As for the new list walking interface, I choose the function "no_equiv" and tried the "checked cast" way. The bad news is that GCC failed to bootstrap with the following change: Index: ira.c =================================================================== --- ira.c (revision 215536) +++ ira.c (working copy) @@ -3242,12 +3242,12 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE void *data ATTRIBUTE_UNUSED) { int regno; - rtx list; + rtx_insn_list *list; if (!REG_P (reg)) return; regno = REGNO (reg); - list = reg_equiv[regno].init_insns; + list = as_a <rtx_insn_list *> (reg_equiv[regno].init_insns); if (list == const0_rtx) return; reg_equiv[regno].init_insns = const0_rtx; @@ -3258,9 +3258,9 @@ 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 = list->next ()) { - rtx insn = XEXP (list, 0); + rtx_insn *insn = list->insn (); remove_note (insn, find_reg_note (insn, REG_EQUIV, NULL_RTX)); } } Error message: .... ... checking for suffix of object files... configure: error: in `/home/yangfei/gcc-devel/build/x86_64-unknown-linux-gnu/libgcc': configure: error: cannot compute suffix of object files: cannot compile See `config.log' for more details. make[2]: *** [configure-stage1-target-libgcc] Error 1 make[2]: Leaving directory `/home/yangfei/gcc-devel/build' make[1]: *** [stage1-bubble] Error 2 make[1]: Leaving directory `/home/yangfei/gcc-devel/build' make: *** [all] Error 2 I think the code change is OK. Anything I missed? Cheers, Felix On Sat, Sep 27, 2014 at 5:03 AM, Jeff Law <l...@redhat.com> wrote: > On 09/26/14 07:57, Felix Yang wrote: >> >> Hi Jeff, >> >> Thanks for the suggestions. I updated the patch accordingly. >> >> 1. Both my employer(Huawei) and I have signed the copyright >> assignments with FSF. >> These assignments are already sent via post two days ago and >> hopefully should reach FSF in one week. >> Maybe it's OK to commit this patch now? > > Not really. It needs to be accepted by the FSF before we can include the > work. > >> >> 2. I am not turning member loop_depth of struct equivalence into >> short integer as GCC API such as bb_loop_depth >> returns a loop's depth as a 32-bit interger. > > There's already other places that assume loops don't nest that deep. Please > go ahead and change it. And no need to explicitly mark the unused bits. > That's just a maintenance nightmare in the long term anyway :-) > > >> >> 3. I find it's kind of difficult to use the new type and >> interfaces for list walking the init_insns list for this patch. >> The type of init_insns list is rtx, not rtl_insn_list *. Seems >> we need to change a lot in order to use the new interface. >> Not clear about the reason why it is not adjusted when we are >> transferring to the new interface. >> Anyway, I think it's better to have another patch fix that issue. >> OK? > > The right way to go is to add a checked cast when we have some code that is > using the old interface and other code using the new interface. It's > actually a pretty easy change. > > The checked casts effectively mark the limits of where we've been able to > push the RTL typesafety work. Long term as we push the typesafety work > further into the compiler many/most of the checked casts will go away. > > Unfortunately, that won't work in this case because other code wants to > store a (const0_rtx) into the insn list. (const0_rtx) isn't an INSN, so the > checked cast fails and we get a nice abort/ICE. > > Conceptually we just need another marker that is an INSN and we might as > well just convert the whole file to use the new interface at that point. > > Consider the request pulled. > > The const0-rtx problem may be why this wasn't converted in the first palce. > Or it may simply have been a time problem. David's done > 250 patches > around RTL typesafety, but he also has other work to be doing ;-) > >> >> 4. This bug is only reproduceable with my local customized GCC >> version. So I don't have a testcase then. > > OK. > > I'll do a final review when I get notice about the copyright assignment from > the FSF. > > jeff >
Index: gcc/ChangeLog =================================================================== --- gcc/ChangeLog (revision 215658) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,14 @@ +2014-09-26 Felix Yang <felix.y...@huawei.com> + Jeff Law <l...@redhat.com> + + * ira.c (struct equivalence): Change member "is_arg_equivalence" and "replace" + into boolean bitfields; turn member "loop_depth" into a short integer; add new + member "no_equiv" and "reserved". + (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-26 Jan Hubicka <hubi...@ucw.cz> PR ipa/60665 Index: gcc/ira.c =================================================================== --- gcc/ira.c (revision 215658) +++ gcc/ira.c (working copy) @@ -2894,12 +2894,14 @@ struct equivalence rtx init_insns; /* Loop depth is used to recognize equivalences which appear to be present within the same loop (or in an inner loop). */ - int loop_depth; + short loop_depth; /* Nonzero if this had a preexisting REG_EQUIV note. */ - int is_arg_equivalence; + unsigned char is_arg_equivalence : 1; /* Set when an attempt should be made to replace a register with the associated src_p entry. */ - char replace; + unsigned char replace : 1; + /* Set if this register has no known equivalence. */ + unsigned char no_equiv : 1; }; /* 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; + + /* If we have already processed this pseudo and determined it + can not have an equivalence, then honor that decision. */ + 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))); @@ -3538,7 +3572,7 @@ update_equiv_regs (void) reg_equiv[regno].replacement = x; reg_equiv[regno].src_p = &SET_SRC (set); - reg_equiv[regno].loop_depth = loop_depth; + reg_equiv[regno].loop_depth = (short) loop_depth; /* Don't mess with things live during setjmp. */ if (REG_LIVE_LENGTH (regno) >= 0 && optimize) @@ -3668,7 +3702,7 @@ update_equiv_regs (void) rtx equiv_insn; if (! reg_equiv[regno].replace - || reg_equiv[regno].loop_depth < loop_depth + || reg_equiv[regno].loop_depth < (short) loop_depth /* There is no sense to move insns if live range shrinkage or register pressure-sensitive scheduling were done because it will not