From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com [IPv6:2a00:1450:4864:20::12e]) by sourceware.org (Postfix) with ESMTPS id E32D43858431 for ; Fri, 17 Mar 2023 08:31:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E32D43858431 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lf1-x12e.google.com with SMTP id g17so5550032lfv.4 for ; Fri, 17 Mar 2023 01:31:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679041882; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=QKy/lK3gdOEsED67+iDFxK+5BxDW0RlUJM2AwgjOXzA=; b=EfyZwsb+opy7kDU4YR/WbphttpWtY5lKoLDZBs6kow0xndIN4Q5keh9/68sUXF3Plq TwfNz+vw6bedNt++u+7qnIYSb92wLuaOK7smG5loKfLG6yR8bZlOqN6r/DlkKK21mjqu MOdSQTNc7Qce7kqNIF5K+wcvXfTyiN3Zl3UlLmJX3sLt6Yx/vdEN1DfwafrQdY2rX9p1 4UvTkJzdj7c56tiRQ1CPwaZH/m4/ijJ+Bj2V+bNfBdrIGviCQKhlvn3TMstOLaCsbuoB ZxDgCwd8Xe1oTdI6Nzx62a+VQApUvBkfYwr9x7or3LaI5D47a0ydwpgNg1yKwR2N7LsY a/7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679041882; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=QKy/lK3gdOEsED67+iDFxK+5BxDW0RlUJM2AwgjOXzA=; b=fA21mtakETYVTIcjkBEJECV6XEN4dHb9HIoQ0RyytWw3Y8KoU0uVfI7bmhBWz76n5/ 5Dw9O1Gz9JCboppDAG3M9uTHRVXZCWFCbCcWcMWScfu7zZUTILGfsEJIDnoMsUOi2YiS UYeeSiIZpOq2dYQ3YyRgHbTXIzCbwqzMw8ElmYHMCkvOc6v6gWsfKu+a82jS1EYbndho EGT2eMiqXLxKwm4NUg+AJsFaw/Qnl0TACXnWrzSVxEwz/YwF836/Dk/95mMRt4sUSOuG 9c49X0OKzAnwM8JpZTevHzQOyuWPzjUt+p6BtbOZEPYcVIDhKehi5ajEs60E4QYHvaMs robw== X-Gm-Message-State: AO0yUKXD0dWygbceLDsOdlN2+W0ViG8rpvz6eAh8QadEo9qSVOSnpHp9 5hDiacGcl/NSCPbMtWD5sE/sGwDBxi+XSBfIxkE= X-Google-Smtp-Source: AK7set9a9ORE6jcyl1qp6qQ/KgAb9BzUFvW9Bb5d2+kLTGToJNwBATji5zgJe5/GF7A41abjzEfVlLo8am0HYLY+0uM= X-Received: by 2002:ac2:434f:0:b0:4dd:a025:d8c with SMTP id o15-20020ac2434f000000b004dda0250d8cmr3940031lfl.5.1679041882063; Fri, 17 Mar 2023 01:31:22 -0700 (PDT) MIME-Version: 1.0 References: <20230316152706.2214124-1-manolis.tsamis@vrull.eu> In-Reply-To: <20230316152706.2214124-1-manolis.tsamis@vrull.eu> From: Richard Biener Date: Fri, 17 Mar 2023 09:31:09 +0100 Message-ID: Subject: Re: [PATCH v1] [RFC] Improve folding for comparisons with zero in tree-ssa-forwprop. To: Manolis Tsamis , Andrew MacLeod Cc: gcc-patches@gcc.gnu.org, Philipp Tomsich Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Mar 16, 2023 at 4:27=E2=80=AFPM Manolis Tsamis wrote: > > For this C testcase: > > void g(); > void f(unsigned int *a) > { > if (++*a =3D=3D 1) > g(); > } > > GCC will currently emit a comparison with 1 by using the value > of *a after the increment. This can be improved by comparing > against 0 and using the value before the increment. As a result > there is a potentially shorter dependancy chain (no need to wait > for the result of +1) and on targets with compare zero instructions > the generated code is one instruction shorter. The downside is we now need two registers and their lifetime overlaps. Your patch mixes changing / inverting a parameter (which seems unneeded for the actual change) with preferring compares against zero. What's the reason to specifically prefer compares against zero? On x86 we have add that sets flags, so ++*a =3D=3D 0 would be preferred, but for your sequence we'd need a test reg, reg; branch on zero, so we do not save any instruction. We do have quite some number of bugreports with regards to making VRPs life harder when splitting things this way. It's easier for VRP to handle _1 =3D _2 + 1; if (_1 =3D=3D 1) than it is _1 =3D _2 + 1; if (_2 =3D=3D 0) where VRP fails to derive a range for _1 on the _2 =3D=3D 0 branch. So bes= ides the life-range issue there's other side-effects as well. Maybe ranger mean= while can handle the above case? What's the overall effect of the change on a larger code base? Thanks, Richard. > > Example from Aarch64: > > Before > ldr w1, [x0] > add w1, w1, 1 > str w1, [x0] > cmp w1, 1 > beq .L4 > ret > > After > ldr w1, [x0] > add w2, w1, 1 > str w2, [x0] > cbz w1, .L4 > ret > > gcc/ChangeLog: > > * tree-ssa-forwprop.cc (combine_cond_expr_cond): > (forward_propagate_into_comparison_1): Optimize > for zero comparisons. > > Signed-off-by: Manolis Tsamis > --- > > gcc/tree-ssa-forwprop.cc | 41 +++++++++++++++++++++++++++------------- > 1 file changed, 28 insertions(+), 13 deletions(-) > > diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc > index e34f0888954..93d5043821b 100644 > --- a/gcc/tree-ssa-forwprop.cc > +++ b/gcc/tree-ssa-forwprop.cc > @@ -373,12 +373,13 @@ rhs_to_tree (tree type, gimple *stmt) > /* Combine OP0 CODE OP1 in the context of a COND_EXPR. Returns > the folded result in a form suitable for COND_EXPR_COND or > NULL_TREE, if there is no suitable simplified form. If > - INVARIANT_ONLY is true only gimple_min_invariant results are > - considered simplified. */ > + ALWAYS_COMBINE is false then only combine it the resulting > + expression is gimple_min_invariant or considered simplified > + compared to the original. */ > > static tree > combine_cond_expr_cond (gimple *stmt, enum tree_code code, tree type, > - tree op0, tree op1, bool invariant_only) > + tree op0, tree op1, bool always_combine) > { > tree t; > > @@ -398,17 +399,31 @@ combine_cond_expr_cond (gimple *stmt, enum tree_cod= e code, tree type, > /* Canonicalize the combined condition for use in a COND_EXPR. */ > t =3D canonicalize_cond_expr_cond (t); > > - /* Bail out if we required an invariant but didn't get one. */ > - if (!t || (invariant_only && !is_gimple_min_invariant (t))) > + if (!t) > { > fold_undefer_overflow_warnings (false, NULL, 0); > return NULL_TREE; > } > > - bool nowarn =3D warning_suppressed_p (stmt, OPT_Wstrict_overflow); > - fold_undefer_overflow_warnings (!nowarn, stmt, 0); > + if (always_combine || is_gimple_min_invariant (t)) > + { > + bool nowarn =3D warning_suppressed_p (stmt, OPT_Wstrict_overflow); > + fold_undefer_overflow_warnings (!nowarn, stmt, 0); > + return t; > + } > > - return t; > + /* If the result of folding is a zero comparison treat it preferential= ly. */ > + if (TREE_CODE_CLASS (TREE_CODE (t)) =3D=3D tcc_comparison > + && integer_zerop (TREE_OPERAND (t, 1)) > + && !integer_zerop (op1)) > + { > + bool nowarn =3D warning_suppressed_p (stmt, OPT_Wstrict_overflow); > + fold_undefer_overflow_warnings (!nowarn, stmt, 0); > + return t; > + } > + > + fold_undefer_overflow_warnings (false, NULL, 0); > + return NULL_TREE; > } > > /* Combine the comparison OP0 CODE OP1 at LOC with the defining statemen= ts > @@ -432,7 +447,7 @@ forward_propagate_into_comparison_1 (gimple *stmt, > if (def_stmt && can_propagate_from (def_stmt)) > { > enum tree_code def_code =3D gimple_assign_rhs_code (def_stmt); > - bool invariant_only_p =3D !single_use0_p; > + bool always_combine =3D single_use0_p; > > rhs0 =3D rhs_to_tree (TREE_TYPE (op1), def_stmt); > > @@ -442,10 +457,10 @@ forward_propagate_into_comparison_1 (gimple *stmt, > && TREE_CODE (TREE_TYPE (TREE_OPERAND (rhs0, 0))) > =3D=3D BOOLEAN_TYPE) > || TREE_CODE_CLASS (def_code) =3D=3D tcc_comparison)) > - invariant_only_p =3D false; > + always_combine =3D true; > > tmp =3D combine_cond_expr_cond (stmt, code, type, > - rhs0, op1, invariant_only_p); > + rhs0, op1, always_combine); > if (tmp) > return tmp; > } > @@ -459,7 +474,7 @@ forward_propagate_into_comparison_1 (gimple *stmt, > { > rhs1 =3D rhs_to_tree (TREE_TYPE (op0), def_stmt); > tmp =3D combine_cond_expr_cond (stmt, code, type, > - op0, rhs1, !single_use1_p); > + op0, rhs1, single_use1_p); > if (tmp) > return tmp; > } > @@ -470,7 +485,7 @@ forward_propagate_into_comparison_1 (gimple *stmt, > && rhs1 !=3D NULL_TREE) > tmp =3D combine_cond_expr_cond (stmt, code, type, > rhs0, rhs1, > - !(single_use0_p && single_use1_p)); > + single_use0_p && single_use1_p); > > return tmp; > } > -- > 2.34.1 >