On 18 June 2014 05:32, Jeff Law <l...@redhat.com> wrote: > On 06/11/14 03:35, Zhenqiang Chen wrote: >> >> >> Thanks for the comments. df_live seams redundant. >> >> With flag_ira_loop_pressure, the pass will call df_analyze () at the >> beginning, which can make sure all the DF info are correct. >> >> Can we guarantee all DF_... correct without df_analyze ()? > > They should be fine in this context. > > > >> +/* Pre-check candidate DEST to skip the one which can not make a valid >> insn >> + during move_invariant_reg. SIMPlE is to skip HARD_REGISTER. */ > > s/SIMPlE/SIMPLE/ > > > >> + { >> + /* Multi definitions at this stage, most likely are due to >> + instruction constrain, which requires both read and >> write > > s/constrain/constraints/ > > Though that doesn't make sense. Constraints don't come into play until much > later in the pipeline. Certainly there's been code in the expanders and > elsewhere to try and make the code we generate more acceptable to 2-address > targets and that's probably what you're really running into. I think the > code is fine, but that you need to improve the comment. > > ISTM that if your primary focus is to filter out read/write operands, then > just say that and ignore the constraints or other mechanisms by which we got > a read/write pseudo. > > So I think with those two small comment changes, this patch is OK for the > trunk. Please post the final version for archival purposes before checking > it in.
Thanks. Patch was installed @r211885 with the two comment changes and one more change to use FOR_EACH_INSN_INFO_DEF other than for (def_rec = DF_INSN_INFO_DEFS (insn_info); *def_rec; def_rec++) Bootstrap and no make check regression on X86-64. Index: gcc/loop-invariant.c =================================================================== --- gcc/loop-invariant.c (revision 211832) +++ gcc/loop-invariant.c (working copy) @@ -839,6 +839,39 @@ return true; } +/* Pre-check candidate DEST to skip the one which can not make a valid insn + during move_invariant_reg. SIMPLE is to skip HARD_REGISTER. */ +static bool +pre_check_invariant_p (bool simple, rtx dest) +{ + if (simple && REG_P (dest) && DF_REG_DEF_COUNT (REGNO (dest)) > 1) + { + df_ref use; + rtx ref; + unsigned int i = REGNO (dest); + struct df_insn_info *insn_info; + df_ref def_rec; + + for (use = DF_REG_USE_CHAIN (i); use; use = DF_REF_NEXT_REG (use)) + { + ref = DF_REF_INSN (use); + insn_info = DF_INSN_INFO_GET (ref); + + FOR_EACH_INSN_INFO_DEF (def_rec, insn_info) + if (DF_REF_REGNO (def_rec) == i) + { + /* Multi definitions at this stage, most likely are due to + instruction constraints, which requires both read and write + on the same register. Since move_invariant_reg is not + powerful enough to handle such cases, just ignore the INV + and leave the chance to others. */ + return false; + } + } + } + return true; +} + /* Finds invariant in INSN. ALWAYS_REACHED is true if the insn is always executed. ALWAYS_EXECUTED is true if the insn is always executed, unless the program ends due to a function call. */ @@ -868,7 +901,8 @@ || HARD_REGISTER_P (dest)) simple = false; - if (!may_assign_reg_p (SET_DEST (set)) + if (!may_assign_reg_p (dest) + || !pre_check_invariant_p (simple, dest) || !check_maybe_invariant (SET_SRC (set))) return;