public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tamar Christina <Tamar.Christina@arm.com>
To: Richard Biener <rguenther@suse.de>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	nd <nd@arm.com>, "jeffreyalaw@gmail.com" <jeffreyalaw@gmail.com>,
	Richard Sandiford <Richard.Sandiford@arm.com>
Subject: Re: [PATCH 1/2]middle-end: RFC: On expansion of conditional branches, give hint if argument is a truth type to backend
Date: Mon, 26 Sep 2022 11:05:26 +0000	[thread overview]
Message-ID: <VI1PR08MB5325CC44E0458DC8148ED695FF529@VI1PR08MB5325.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <nycvar.YFH.7.77.849.2209261029530.6652@jbgna.fhfr.qr>

[-- Attachment #1: Type: text/plain, Size: 5629 bytes --]

> > This pattern occurs more than 120,000 times in SPECCPU 2017 and so is quite common.

> How does this help a target?

So the idea is to communicate that only the bottom bit of the value is relevant and not the entire value itself.

> Why does RTL nonzerop bits not recover thisinformation and the desired optimization done later during combinefor example?

I'm not sure it works here. We (AArch64) don't have QImode integer registers, so our apcs says that the top bits of the 32-bit registers it's passed in are undefined.

We have to zero extend the value first if we really want it as an 8-bit value. So the problem is if you e.g. Pass a boolean as an argument of a function I don't think nonzero bits will return anything useful.

> Why's a SImode compare not OK if there's no QImode compare?

The mode then becomes irrelevant because we're telling the backend that only a single bit matters. And we have instructions to test and branch on the value of a single bit. See https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602090.html for the testcases

> We have undocumented addcc, negcc, etc. patterns, should we have aandcc pattern for this indicating support for andcc + jump as opposedto cmpcc + jump?

This could work yeah. I didn't know these existed.

> So - what's the target and what's a testcase?

See https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602090.html :)

Thanks,
Tamar

________________________________
From: Richard Biener <rguenther@suse.de>
Sent: Monday, September 26, 2022 11:35 AM
To: Tamar Christina <Tamar.Christina@arm.com>
Cc: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>; nd <nd@arm.com>; jeffreyalaw@gmail.com <jeffreyalaw@gmail.com>; Richard Sandiford <Richard.Sandiford@arm.com>
Subject: Re: [PATCH 1/2]middle-end: RFC: On expansion of conditional branches, give hint if argument is a truth type to backend

On Fri, 23 Sep 2022, Tamar Christina wrote:

> Hi All,
>
> This is an RFC to figure out how to deal with targets that don't have native
> comparisons against QImode values.
>
> Booleans, at least in C99 and higher are 0-1 valued.  This means that we only
> really need to test a single bit.  However in RTL we no longer have this
> information available and just have an SImode value (due to the promotion of
> QImode to SImode).
>
> This RFC fixes it by emitting an explicit & 1 during the expansion of the
> conditional branch.
>
> However it's unlikely that we want to do this unconditionally.  Most targets
> I've tested seem to have harmless code changes, like x86 changes from testb to
> andl, $1.
>
> So I have two questions:
>
> 1. Should I limit this behind a target macro? Or should I just leave it for all
>    targets and deal with the fallout.
> 2. How can I tell whether the C99 0-1 values bools are being used or the older
>    0, non-0 variant?
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> However there are some benign codegen changes on x86, testb changed to andl $1.
>
> This pattern occurs more than 120,000 times in SPECCPU 2017 and so is quite common.

How does this help a target?  Why does RTL nonzerop bits not recover this
information and the desired optimization done later during combine
for example?  Why's a SImode compare not OK if there's no QImode compare?
We have undocumented addcc, negcc, etc. patterns, should we have a
andcc pattern for this indicating support for andcc + jump as opposed
to cmpcc + jump?

So - what's the target and what's a testcase?

Thanks,
Richard.

> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
>        * tree.h (tree_zero_one_valued_p): New.
>        * dojump.cc (do_jump): Add & 1 if truth type.
>
> --- inline copy of patch --
> diff --git a/gcc/dojump.cc b/gcc/dojump.cc
> index 2af0cd1aca3b6af13d5d8799094ee93f18022296..8eaf1be49cd12298e61c6946ae79ca9de6197864 100644
> --- a/gcc/dojump.cc
> +++ b/gcc/dojump.cc
> @@ -605,7 +605,17 @@ do_jump (tree exp, rtx_code_label *if_false_label,
>        /* Fall through and generate the normal code.  */
>      default:
>      normal:
> -      temp = expand_normal (exp);
> +      tree cmp = exp;
> +      /* If the expression is a truth type then explicitly generate an & 1
> +      to indicate to the target that it's a zero-one values type.  This
> +      allows the target to further optimize the comparison should it
> +      choose to.  */
> +      if (tree_zero_one_valued_p (exp))
> +     {
> +       type = TREE_TYPE (exp);
> +       cmp = build2 (BIT_AND_EXPR, type, exp, build_int_cstu (type, 1));
> +     }
> +      temp = expand_normal (cmp);
>        do_pending_stack_adjust ();
>        /* The RTL optimizers prefer comparisons against pseudos.  */
>        if (GET_CODE (temp) == SUBREG)
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 8f8a9660c9e0605eb516de194640b8c1b531b798..be3d2dee82f692e81082cf21c878c10f9fe9e1f1 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -4690,6 +4690,7 @@ extern tree signed_or_unsigned_type_for (int, tree);
>  extern tree signed_type_for (tree);
>  extern tree unsigned_type_for (tree);
>  extern bool is_truth_type_for (tree, tree);
> +extern bool tree_zero_one_valued_p (tree);
>  extern tree truth_type_for (tree);
>  extern tree build_pointer_type_for_mode (tree, machine_mode, bool);
>  extern tree build_pointer_type (tree);
>
>
>
>
>

--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

  reply	other threads:[~2022-09-26 11:05 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-23  9:24 Tamar Christina
2022-09-23  9:25 ` [PATCH 2/2]AArch64 Extend tbz pattern to allow SI to SI extensions Tamar Christina
2022-09-23  9:42   ` Richard Sandiford
2022-09-23  9:48     ` Tamar Christina
2022-09-26 10:35 ` [PATCH 1/2]middle-end: RFC: On expansion of conditional branches, give hint if argument is a truth type to backend Richard Biener
2022-09-26 11:05   ` Tamar Christina [this message]
2022-09-26 11:32     ` Richard Biener
2022-09-26 11:46       ` Tamar Christina
2022-09-26 12:34         ` Richard Biener
2022-09-26 12:43           ` Richard Biener
2022-09-26 14:02             ` Tamar Christina
2022-09-28 15:04         ` Richard Sandiford
2022-09-28 17:23           ` Jeff Law
2022-09-29  9:37             ` Richard Sandiford
2022-09-29  9:40               ` Richard Biener
2022-09-29 10:21                 ` Tamar Christina
2022-09-29 11:09                   ` Richard Biener
2022-09-30  8:00                     ` Tamar Christina
2022-09-30  8:28                       ` Richard Sandiford
2022-09-30  8:38                         ` Tamar Christina
2022-09-30  8:48                           ` Richard Sandiford
2022-09-30  9:15                             ` Tamar Christina
2022-09-30 10:16                               ` Richard Biener
2022-09-30 11:11                                 ` Tamar Christina
2022-09-30 11:52                                   ` Richard Biener
2022-09-30 12:48                                     ` Tamar Christina
2022-09-30 14:28                                       ` Richard Sandiford
2022-09-30 14:33                                         ` Richard Biener
2022-09-29 20:49               ` Jeff Law
2022-10-27  3:22 ` Andrew Pinski

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=VI1PR08MB5325CC44E0458DC8148ED695FF529@VI1PR08MB5325.eurprd08.prod.outlook.com \
    --to=tamar.christina@arm.com \
    --cc=Richard.Sandiford@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jeffreyalaw@gmail.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).