Re: [PATCH 1/2]middle-end: RFC: On expansion of conditional branches, give hint if argument is a truth type to backend

[Tamar Christina <Tamar.Christina@arm.com>]

[-- 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)