From: David Edelsohn <dje.gcc@gmail.com>
To: HAO CHEN GUI <guihaoc@linux.ibm.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>,
Segher Boessenkool <segher@kernel.crashing.org>,
"Kewen.Lin" <linkw@linux.ibm.com>,
Peter Bergner <bergner@linux.ibm.com>
Subject: Re: [PATCHv2, rs6000] Enable have_cbranchcc4 on rs6000
Date: Thu, 17 Nov 2022 08:24:48 -0500 [thread overview]
Message-ID: <CAGWvnym54qRGozi_6_y2Tm3YkSc7AHVPiMQ7nyuGe9B9+=nbAQ@mail.gmail.com> (raw)
In-Reply-To: <438c6628-0b9c-e5d0-e198-2fd6edd16a93@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 4215 bytes --]
On Thu, Nov 17, 2022 at 1:39 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:
> Hi,
> The patch enables have_cbrnachcc4 which is a flag in ifcvt.cc to
> indicate if branch by CC bits is invalid or not. The new expand pattern
> "cbranchcc4" is created which intend to match the pattern defined in
> "*cbranch", "*cbranch_2insn" and "*creturn". The operand sequence in
> "cbranchcc4" is inline with the definition in gccint. And the operand
> sequence doesn't matter in pattern matching. So I think it should work.
>
> Compared to last version, one new predicate and one new expander are
> created.
>
> Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
> Is this okay for trunk? Any recommendations? Thanks a lot.
>
> ChangeLog
> 2022-11-17 Haochen Gui <guihaoc@linux.ibm.com>
>
> gcc/
> * config/rs6000/predicates.md (all_branch_comparison_operator):
> New,
> and includes operators in branch_comparison_operator and
> extra_insn_branch_comparison_operator.
> * config/rs6000/rs6000.md (cbranchcc4): New expand pattern.
>
> gcc/testsuite/
> * gcc.target/powerpc/cbranchcc4.c: New.
>
>
> patch.diff
> diff --git a/gcc/config/rs6000/predicates.md
> b/gcc/config/rs6000/predicates.md
> index b1fcc69bb60..843b6f39b84 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -1308,6 +1308,7 @@ (define_special_predicate "equality_operator"
>
> ;; Return 1 if OP is a comparison operation that is valid for a branch
> ;; instruction. We check the opcode against the mode of the CC value.
> +
> ;; validate_condition_mode is an assertion.
> (define_predicate "branch_comparison_operator"
> (and (match_operand 0 "comparison_operator")
> @@ -1331,6 +1332,11 @@ (define_predicate
> "extra_insn_branch_comparison_operator"
> GET_MODE (XEXP (op, 0))),
> 1")))
>
> +;; Return 1 if OP is a comparison operation that is valid for a branch.
> +(define_predicate "all_branch_comparison_operator"
> + (ior (match_operand 0 "branch_comparison_operator")
> + (match_operand 0 "extra_insn_branch_comparison_operator")))
> +
> ;; Return 1 if OP is an unsigned comparison operator.
> (define_predicate "unsigned_comparison_operator"
> (match_code "ltu,gtu,leu,geu"))
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index e9e5cd1e54d..7b7d747a85d 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -13067,6 +13067,16 @@ (define_insn_and_split "*<code><mode>_cc"
> ;; Conditional branches.
> ;; These either are a single bc insn, or a bc around a b.
>
> +(define_expand "cbranchcc4"
> + [(set (pc)
> + (if_then_else (match_operator 0 "all_branch_comparison_operator"
> + [(match_operand 1 "cc_reg_operand")
> + (match_operand 2 "zero_constant")])
> + (label_ref (match_operand 3))
> + (pc)))]
> + ""
> + "")
> +
>
This is better, but the pattern should be near and after the existing
cbranch<mode>4 patterns earlier in the file, not the *cbranch pattern. It
doesn't match the comment.
Why are you using zero_constant predicate instead of matching (const_int 0)
for operand 2?
Why does this need the new all_branch_comparison_operator? Can the ifcvt
optimization correctly elide the 2 insn sequence?
Thanks, David
> (define_insn "*cbranch"
> [(set (pc)
> (if_then_else (match_operator 1 "branch_comparison_operator"
> diff --git a/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c
> b/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c
> new file mode 100644
> index 00000000000..528ba1a878d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-rtl-ce1" } */
> +/* { dg-final { scan-rtl-dump "noce_try_store_flag_constants" "ce1" } } */
> +
> +/* The inner branch should be detected by ifcvt then be converted to a
> setcc
> + with a plus by noce_try_store_flag_constants. */
> +
> +int test (unsigned int a, unsigned int b)
> +{
> + return (a < b ? 0 : (a > b ? 2 : 1));
> +}
>
next prev parent reply other threads:[~2022-11-17 13:25 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-17 6:39 HAO CHEN GUI
2022-11-17 13:24 ` David Edelsohn [this message]
2022-11-18 6:35 ` HAO CHEN GUI
2022-11-18 12:18 ` Segher Boessenkool
2022-11-18 12:36 ` David Edelsohn
2022-11-21 6:18 ` HAO CHEN GUI
2022-11-21 23:49 ` Segher Boessenkool
2022-11-22 7:50 ` HAO CHEN GUI
2022-11-22 3:11 ` Kewen.Lin
2022-11-22 5:12 ` HAO CHEN GUI
2022-11-22 6:02 ` Kewen.Lin
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='CAGWvnym54qRGozi_6_y2Tm3YkSc7AHVPiMQ7nyuGe9B9+=nbAQ@mail.gmail.com' \
--to=dje.gcc@gmail.com \
--cc=bergner@linux.ibm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=guihaoc@linux.ibm.com \
--cc=linkw@linux.ibm.com \
--cc=segher@kernel.crashing.org \
/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).