From: Tamar Christina <Tamar.Christina@arm.com>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Jonathan Wakely <jwakely@redhat.com>,
Richard Biener <rguenther@suse.de>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
nd <nd@arm.com>
Subject: RE: [PATCH] middle-end: fix de-optimizations with bitclear patterns on signed values
Date: Fri, 19 Nov 2021 08:52:11 +0000 [thread overview]
Message-ID: <AM0PR08MB53164E8EC8BFAD9811C74A11FF9C9@AM0PR08MB5316.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <AM0PR08MB5316B2FB8E5A2A0D5229DF64FF959@AM0PR08MB5316.eurprd08.prod.outlook.com>
Ping
> -----Original Message-----
> From: Tamar Christina
> Sent: Friday, November 12, 2021 7:31 AM
> To: Jakub Jelinek <jakub@redhat.com>
> Cc: Jonathan Wakely <jwakely@redhat.com>; Richard Biener
> <rguenther@suse.de>; gcc-patches@gcc.gnu.org; nd <nd@arm.com>
> Subject: RE: [PATCH] middle-end: fix de-optimizations with bitclear patterns
> on signed values
>
>
>
> > -----Original Message-----
> > From: Jakub Jelinek <jakub@redhat.com>
> > Sent: Thursday, November 4, 2021 4:11 PM
> > To: Tamar Christina <Tamar.Christina@arm.com>
> > Cc: Jonathan Wakely <jwakely@redhat.com>; Richard Biener
> > <rguenther@suse.de>; gcc-patches@gcc.gnu.org; nd <nd@arm.com>
> > Subject: Re: [PATCH] middle-end: fix de-optimizations with bitclear
> > patterns on signed values
> >
> > On Thu, Nov 04, 2021 at 12:19:34PM +0000, Tamar Christina wrote:
> > > I'm not sure the precision matters since if the conversion resulted
> > > in not enough precision such that It influences the compare it would
> > > have
> > been optimized out.
> >
> > You can't really rely on other optimizations being performed. They
> > will usually happen, but might not because such code only materialized
> > short time ago without folding happening in between, or some debug
> > counters or -
> > fno-* disabling some passes, ...
>
> Fair point, I have separated out the logic as you requested and added the
> debug fix.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
> and no regressions.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.c (spaceship_replacement): Handle new canonical
> codegen.
>
> --- inline copy of patch ---
>
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c index
> 0e339c46afa29fa97f90d9bc4394370cd9b4b396..3ad5b23885a37eec0beff229e2
> a96e86658b2d1a 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -2038,11 +2038,36 @@ spaceship_replacement (basic_block cond_bb,
> basic_block middle_bb,
> gimple *orig_use_stmt = use_stmt;
> tree orig_use_lhs = NULL_TREE;
> int prec = TYPE_PRECISION (TREE_TYPE (phires));
> - if (is_gimple_assign (use_stmt)
> - && gimple_assign_rhs_code (use_stmt) == BIT_AND_EXPR
> - && TREE_CODE (gimple_assign_rhs2 (use_stmt)) == INTEGER_CST
> - && (wi::to_wide (gimple_assign_rhs2 (use_stmt))
> - == wi::shifted_mask (1, prec - 1, false, prec)))
> + bool is_cast = false;
> +
> + /* Deal with the case when match.pd has rewritten the (res & ~1) == 0
> + into res <= 1 and has left a type-cast for signed types. */
> + if (gimple_assign_cast_p (use_stmt))
> + {
> + orig_use_lhs = gimple_assign_lhs (use_stmt);
> + /* match.pd would have only done this for a signed type,
> + so the conversion must be to an unsigned one. */
> + tree ty1 = TREE_TYPE (gimple_assign_rhs1 (use_stmt));
> + tree ty2 = TREE_TYPE (orig_use_lhs);
> +
> + if (!TYPE_UNSIGNED (ty2) || !INTEGRAL_TYPE_P (ty2))
> + return false;
> + if (TYPE_PRECISION (ty1) != TYPE_PRECISION (ty2))
> + return false;
> + if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (orig_use_lhs))
> + return false;
> + if (EDGE_COUNT (phi_bb->preds) != 4)
> + return false;
> + if (!single_imm_use (orig_use_lhs, &use_p, &use_stmt))
> + return false;
> +
> + is_cast = true;
> + }
> + else if (is_gimple_assign (use_stmt)
> + && gimple_assign_rhs_code (use_stmt) == BIT_AND_EXPR
> + && TREE_CODE (gimple_assign_rhs2 (use_stmt)) == INTEGER_CST
> + && (wi::to_wide (gimple_assign_rhs2 (use_stmt))
> + == wi::shifted_mask (1, prec - 1, false, prec)))
> {
> /* For partial_ordering result operator>= with unspec as second
> argument is (res & 1) == res, folded by match.pd into @@ -2099,7
> +2124,7 @@ spaceship_replacement (basic_block cond_bb, basic_block
> middle_bb,
> || !tree_fits_shwi_p (rhs)
> || !IN_RANGE (tree_to_shwi (rhs), -1, 1))
> return false;
> - if (orig_use_lhs)
> + if (orig_use_lhs && !is_cast)
> {
> if ((cmp != EQ_EXPR && cmp != NE_EXPR) || !integer_zerop (rhs))
> return false;
> @@ -2310,62 +2335,101 @@ spaceship_replacement (basic_block cond_bb,
> basic_block middle_bb,
> one_cmp = GT_EXPR;
>
> enum tree_code res_cmp;
> - switch (cmp)
> +
> + if (is_cast)
> {
> - case EQ_EXPR:
> - if (integer_zerop (rhs))
> - res_cmp = EQ_EXPR;
> - else if (integer_minus_onep (rhs))
> - res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
> - else if (integer_onep (rhs))
> - res_cmp = one_cmp;
> - else
> + if (TREE_CODE (rhs) != INTEGER_CST)
> return false;
> - break;
> - case NE_EXPR:
> - if (integer_zerop (rhs))
> - res_cmp = NE_EXPR;
> - else if (integer_minus_onep (rhs))
> - res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
> - else if (integer_onep (rhs))
> - res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
> - else
> - return false;
> - break;
> - case LT_EXPR:
> - if (integer_onep (rhs))
> - res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
> - else if (integer_zerop (rhs))
> - res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
> - else
> - return false;
> - break;
> - case LE_EXPR:
> - if (integer_zerop (rhs))
> - res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
> - else if (integer_minus_onep (rhs))
> - res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
> - else
> - return false;
> - break;
> - case GT_EXPR:
> - if (integer_minus_onep (rhs))
> - res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
> - else if (integer_zerop (rhs))
> - res_cmp = one_cmp;
> - else
> - return false;
> - break;
> - case GE_EXPR:
> - if (integer_zerop (rhs))
> - res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
> - else if (integer_onep (rhs))
> - res_cmp = one_cmp;
> - else
> - return false;
> - break;
> - default:
> - gcc_unreachable ();
> + /* As for -ffast-math we assume the 2 return to be
> + impossible, canonicalize (unsigned) res <= 1U or
> + (unsigned) res < 2U into res >= 0 and (unsigned) res > 1U
> + or (unsigned) res >= 2U as res < 0. */
> + switch (cmp)
> + {
> + case LE_EXPR:
> + if (!integer_onep (rhs))
> + return false;
> + res_cmp = GE_EXPR;
> + break;
> + case LT_EXPR:
> + if (wi::ne_p (wi::to_widest (rhs), 2))
> + return false;
> + res_cmp = GE_EXPR;
> + break;
> + case GT_EXPR:
> + if (!integer_onep (rhs))
> + return false;
> + res_cmp = LT_EXPR;
> + break;
> + case GE_EXPR:
> + if (wi::ne_p (wi::to_widest (rhs), 2))
> + return false;
> + res_cmp = LT_EXPR;
> + break;
> + default:
> + return false;
> + }
> + rhs = build_zero_cst (TREE_TYPE (phires));
> + }
> + else
> + {
> + switch (cmp)
> + {
> + case EQ_EXPR:
> + if (integer_zerop (rhs))
> + res_cmp = EQ_EXPR;
> + else if (integer_minus_onep (rhs))
> + res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
> + else if (integer_onep (rhs))
> + res_cmp = one_cmp;
> + else
> + return false;
> + break;
> + case NE_EXPR:
> + if (integer_zerop (rhs))
> + res_cmp = NE_EXPR;
> + else if (integer_minus_onep (rhs))
> + res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
> + else if (integer_onep (rhs))
> + res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
> + else
> + return false;
> + break;
> + case LT_EXPR:
> + if (integer_onep (rhs))
> + res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
> + else if (integer_zerop (rhs))
> + res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
> + else
> + return false;
> + break;
> + case LE_EXPR:
> + if (integer_zerop (rhs))
> + res_cmp = one_cmp == LT_EXPR ? GE_EXPR : LE_EXPR;
> + else if (integer_minus_onep (rhs))
> + res_cmp = one_cmp == LT_EXPR ? GT_EXPR : LT_EXPR;
> + else
> + return false;
> + break;
> + case GT_EXPR:
> + if (integer_minus_onep (rhs))
> + res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
> + else if (integer_zerop (rhs))
> + res_cmp = one_cmp;
> + else
> + return false;
> + break;
> + case GE_EXPR:
> + if (integer_zerop (rhs))
> + res_cmp = one_cmp == LT_EXPR ? LE_EXPR : GE_EXPR;
> + else if (integer_onep (rhs))
> + res_cmp = one_cmp;
> + else
> + return false;
> + break;
> + default:
> + gcc_unreachable ();
> + }
> }
>
> if (gimple_code (use_stmt) == GIMPLE_COND) @@ -2394,6 +2458,7 @@
> spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
> use_operand_p use_p;
> imm_use_iterator iter;
> bool has_debug_uses = false;
> + bool has_cast_debug_uses = false;
> FOR_EACH_IMM_USE_FAST (use_p, iter, phires)
> {
> gimple *use_stmt = USE_STMT (use_p); @@ -2405,12 +2470,14 @@
> spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
> }
> if (orig_use_lhs)
> {
> - if (!has_debug_uses)
> + if (!has_debug_uses || is_cast)
> FOR_EACH_IMM_USE_FAST (use_p, iter, orig_use_lhs)
> {
> gimple *use_stmt = USE_STMT (use_p);
> gcc_assert (is_gimple_debug (use_stmt));
> has_debug_uses = true;
> + if (is_cast)
> + has_cast_debug_uses = true;
> }
> gimple_stmt_iterator gsi = gsi_for_stmt (orig_use_stmt);
> tree zero = build_zero_cst (TREE_TYPE (orig_use_lhs)); @@ -2448,7
> +2515,23 @@ spaceship_replacement (basic_block cond_bb, basic_block
> middle_bb,
> gsi_insert_before (&gsi, g, GSI_SAME_STMT);
> replace_uses_by (phires, temp2);
> if (orig_use_lhs)
> - replace_uses_by (orig_use_lhs, temp2);
> + {
> + if (has_cast_debug_uses)
> + {
> + tree temp3 = make_node (DEBUG_EXPR_DECL);
> + DECL_ARTIFICIAL (temp3) = 1;
> + TREE_TYPE (temp3) = TREE_TYPE (orig_use_lhs);
> + SET_DECL_MODE (temp3, TYPE_MODE (type));
> + t = build2 (EQ_EXPR, boolean_type_node, lhs1, rhs2);
> + t = build3 (COND_EXPR, type, t, build_zero_cst (type),
> + temp1);
> + g = gimple_build_debug_bind (temp3, t, phi);
> + gsi_insert_before (&gsi, g, GSI_SAME_STMT);
> + replace_uses_by (orig_use_lhs, temp3);
> + }
> + else
> + replace_uses_by (orig_use_lhs, temp2);
> + }
> }
> }
next prev parent reply other threads:[~2021-11-19 8:52 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-15 11:08 Tamar Christina
2021-10-15 11:30 ` Richard Biener
2021-10-25 14:26 ` Tamar Christina
2021-10-26 8:26 ` Richard Biener
2021-10-26 8:35 ` Tamar Christina
2021-10-26 8:45 ` Richard Biener
2021-10-26 12:10 ` Tamar Christina
2021-10-26 13:13 ` Richard Biener
2021-10-26 13:20 ` Jakub Jelinek
2021-10-26 13:21 ` Richard Biener
2021-10-26 13:36 ` Jakub Jelinek
2021-10-26 13:38 ` Richard Biener
2021-10-26 19:35 ` Jonathan Wakely
2021-10-26 19:39 ` Jakub Jelinek
2021-10-26 19:50 ` Jonathan Wakely
2021-11-03 10:56 ` Tamar Christina
2021-11-03 14:20 ` Jakub Jelinek
2021-11-04 12:19 ` Tamar Christina
2021-11-04 15:10 ` Jakub Jelinek
2021-11-12 7:30 ` Tamar Christina
2021-11-19 8:52 ` Tamar Christina [this message]
2021-11-19 11:19 ` Jakub Jelinek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AM0PR08MB53164E8EC8BFAD9811C74A11FF9C9@AM0PR08MB5316.eurprd08.prod.outlook.com \
--to=tamar.christina@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=jwakely@redhat.com \
--cc=nd@arm.com \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).