From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 61567 invoked by alias); 23 Oct 2018 12:02:53 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 61536 invoked by uid 89); 23 Oct 2018 12:02:53 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Lyon, lyon, Christophe X-HELO: mail-vk1-f194.google.com Received: from mail-vk1-f194.google.com (HELO mail-vk1-f194.google.com) (209.85.221.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 23 Oct 2018 12:02:50 +0000 Received: by mail-vk1-f194.google.com with SMTP id x78-v6so251994vke.11 for ; Tue, 23 Oct 2018 05:02:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=uWjOiY8mThC3D9W9nqj1/THrsHBpV7vp2tVQz1n0efQ=; b=E4fzuh+Dufbylh7YhJIBkwrsw5pRVbFGUAjutChhN+stKdeGTCHTUu8JHviz8rD6tr /t66+CRdmNX4HZqU80dGQps46uuwbfD2f0yJCiKmpDqPZl2vMRafL1yvALYbrHOMRxDV vooIn/P+9XKSidQvaGzZaXGQe1XwfWptf6sx0= MIME-Version: 1.0 References: <68abf72a5400b96b9a100966331d3ad2056648e7.1540237620.git.segher@kernel.crashing.org> In-Reply-To: From: Christophe Lyon Date: Tue, 23 Oct 2018 12:36:00 -0000 Message-ID: Subject: Re: [PATCH] combine: Do not combine moves from hard registers To: Segher Boessenkool Cc: gcc Patches , bergner@linux.ibm.com Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-10/txt/msg01423.txt.bz2 On Tue, 23 Oct 2018 at 12:14, Christophe Lyon wrote: > > On Mon, 22 Oct 2018 at 22:17, Segher Boessenkool > 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 > > > > 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 > >