On Tue, 23 Oct 2018 at 12:14, Christophe Lyon <christophe.l...@linaro.org> wrote: > > On Mon, 22 Oct 2018 at 22:17, Segher Boessenkool > <seg...@kernel.crashing.org> wrote: > > > > On most targets every function starts with moves from the parameter > > passing (hard) registers into pseudos. Similarly, after every call > > there is a move from the return register into a pseudo. These moves > > usually combine with later instructions (leaving pretty much the same > > instruction, just with a hard reg instead of a pseudo). > > > > This isn't a good idea. Register allocation can get rid of unnecessary > > moves just fine, and moving the parameter passing registers into many > > later instructions tends to prevent good register allocation. This > > patch disallows combining moves from a hard (non-fixed) register. > > > > This also avoid the problem mentioned in PR87600 #c3 (combining hard > > registers into inline assembler is problematic). > > > > Because the register move can often be combined with other instructions > > *itself*, for example for setting some condition code, this patch adds > > extra copies via new pseudos after every copy-from-hard-reg. > > > > On some targets this reduces average code size. On others it increases > > it a bit, 0.1% or 0.2% or so. (I tested this on all *-linux targets). > > > > I'll commit this to trunk now. If there are problems, please don't > > hesitate to let me know! Thanks. > > > > Hi, > > I have noticed many regressions on arm and aarch64 between 265366 and > 265408 (this commit is 265398). > > I bisected at least one to this commit on aarch64: > FAIL: gcc.dg/ira-shrinkwrap-prep-1.c scan-rtl-dump ira "Split > live-range of register" > The same test also regresses on arm. > > For a whole picture of all the regressions I noticed during these two > commits, have a look at: > http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/265408/report-build-info.html > > Christophe >
I also bisected regressions on arm: gcc.c-torture/execute/920428-2.c gfortran.dg/actual_array_substr_2.f90 both point to this commit too. > > > > > > Segher > > > > > > 2018-10-22 Segher Boessenkool <seg...@kernel.crashing.org> > > > > PR rtl-optimization/87600 > > * combine.c: Add include of expr.h. > > (cant_combine_insn_p): Do not combine moves from any hard non-fixed > > register to a pseudo. > > (make_more_copies): New function, add a copy to a new pseudo after > > the moves from hard registers into pseudos. > > (rest_of_handle_combine): Declare rebuild_jump_labels_after_combine > > later. Call make_more_copies. > > > > --- > > gcc/combine.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 46 insertions(+), 4 deletions(-) > > > > diff --git a/gcc/combine.c b/gcc/combine.c > > index 256b5a4..3ff1760 100644 > > --- a/gcc/combine.c > > +++ b/gcc/combine.c > > @@ -99,6 +99,7 @@ along with GCC; see the file COPYING3. If not see > > #include "explow.h" > > #include "insn-attr.h" > > #include "rtlhooks-def.h" > > +#include "expr.h" > > #include "params.h" > > #include "tree-pass.h" > > #include "valtrack.h" > > @@ -2348,8 +2349,7 @@ cant_combine_insn_p (rtx_insn *insn) > > dest = SUBREG_REG (dest); > > if (REG_P (src) && REG_P (dest) > > && ((HARD_REGISTER_P (src) > > - && ! TEST_HARD_REG_BIT (fixed_reg_set, REGNO (src)) > > - && targetm.class_likely_spilled_p (REGNO_REG_CLASS (REGNO > > (src)))) > > + && ! TEST_HARD_REG_BIT (fixed_reg_set, REGNO (src))) > > || (HARD_REGISTER_P (dest) > > && ! TEST_HARD_REG_BIT (fixed_reg_set, REGNO (dest)) > > && targetm.class_likely_spilled_p (REGNO_REG_CLASS (REGNO > > (dest)))))) > > @@ -14969,11 +14969,53 @@ dump_combine_total_stats (FILE *file) > > total_attempts, total_merges, total_extras, total_successes); > > } > > > > +/* Make pseudo-to-pseudo copies after every hard-reg-to-pseudo-copy, > > because > > + the reg-to-reg copy can usefully combine with later instructions, but we > > + do not want to combine the hard reg into later instructions, for that > > + restricts register allocation. */ > > +static void > > +make_more_copies (void) > > +{ > > + basic_block bb; > > + > > + FOR_EACH_BB_FN (bb, cfun) > > + { > > + rtx_insn *insn; > > + > > + FOR_BB_INSNS (bb, insn) > > + { > > + if (!NONDEBUG_INSN_P (insn)) > > + continue; > > + > > + rtx set = single_set (insn); > > + if (!set) > > + continue; > > + rtx src = SET_SRC (set); > > + rtx dest = SET_DEST (set); > > + if (GET_CODE (src) == SUBREG) > > + src = SUBREG_REG (src); > > + if (!(REG_P (src) && HARD_REGISTER_P (src))) > > + continue; > > + if (TEST_HARD_REG_BIT (fixed_reg_set, REGNO (src))) > > + continue; > > + > > + rtx new_reg = gen_reg_rtx (GET_MODE (dest)); > > + rtx_insn *insn1 = gen_move_insn (new_reg, src); > > + rtx_insn *insn2 = gen_move_insn (dest, new_reg); > > + emit_insn_after (insn1, insn); > > + emit_insn_after (insn2, insn1); > > + delete_insn (insn); > > + > > + insn = insn2; > > + } > > + } > > +} > > + > > /* Try combining insns through substitution. */ > > static unsigned int > > rest_of_handle_combine (void) > > { > > - int rebuild_jump_labels_after_combine; > > + make_more_copies (); > > > > df_set_flags (DF_LR_RUN_DCE + DF_DEFER_INSN_RESCAN); > > df_note_add_problem (); > > @@ -14982,7 +15024,7 @@ rest_of_handle_combine (void) > > regstat_init_n_sets_and_refs (); > > reg_n_sets_max = max_reg_num (); > > > > - rebuild_jump_labels_after_combine > > + int rebuild_jump_labels_after_combine > > = combine_instructions (get_insns (), max_reg_num ()); > > > > /* Combining insns may have turned an indirect jump into a > > -- > > 1.8.3.1 > >