From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 114626 invoked by alias); 23 Oct 2018 10:14:43 -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 114157 invoked by uid 89); 23 Oct 2018 10:14:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.2 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=picture X-HELO: mail-vs1-f66.google.com Received: from mail-vs1-f66.google.com (HELO mail-vs1-f66.google.com) (209.85.217.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 23 Oct 2018 10:14:40 +0000 Received: by mail-vs1-f66.google.com with SMTP id a202so480108vsd.5 for ; Tue, 23 Oct 2018 03:14:40 -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=c5WauTVWKduG4eWvCiCIDKqd+BbRhd7Qql15J8pQ75E=; b=J5wZyCN2gH9QDiM14qd7XDXjAlLtX9kAE2+1dqUmkFMlLj6OzNGu4SLjUiOyZJ0/4A eOdcq5Y+oODPAyfGdSd/bbPamx2CztXB/DQpqzSoPtvi6QqYn+OftpjtwaN0oZjyTLAX v8mLzZzraGqPO/grqED9bqrk+/Fu3M/l78xqc= MIME-Version: 1.0 References: <68abf72a5400b96b9a100966331d3ad2056648e7.1540237620.git.segher@kernel.crashing.org> In-Reply-To: <68abf72a5400b96b9a100966331d3ad2056648e7.1540237620.git.segher@kernel.crashing.org> From: Christophe Lyon Date: Tue, 23 Oct 2018 10:54: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/msg01411.txt.bz2 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 > > 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 >