From: Uros Bizjak <ubizjak@gmail.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] x86: Replace ne:CCC/ne:CCO with UNSPEC_CC_NE in neg patterns
Date: Thu, 27 Oct 2022 08:06:00 +0200 [thread overview]
Message-ID: <CAFULd4ZzZ6S3dig8YhD4Soqx+nVspE=zsP5Q3tHv_6nDhuBEJg@mail.gmail.com> (raw)
In-Reply-To: <20221026185857.234023-1-hjl.tools@gmail.com>
On Wed, Oct 26, 2022 at 8:59 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> In i386.md, neg patterns which set MODE_CC register like
>
> (set (reg:CCC FLAGS_REG)
> (ne:CCC (match_operand:SWI48 1 "general_reg_operand") (const_int 0)))
>
> can lead to errors when operand 1 is a constant value. If FLAGS_REG in
>
> (set (reg:CCC FLAGS_REG)
> (ne:CCC (const_int 2) (const_int 0)))
>
> is set to 1, RTX simplifiers may simplify
>
> (set (reg:SI 93)
> (neg:SI (ltu:SI (reg:CCC 17 flags) (const_int 0 [0]))))
>
> as
>
> (set (reg:SI 93)
> (neg:SI (ltu:SI (const_int 1) (const_int 0 [0]))))
>
> which leads to incorrect results since LTU on MODE_CC register isn't the
> same as "unsigned less than" in x86 backend. To prevent RTL optimizers
> from setting MODE_CC register to a constant, use UNSPEC_CC_NE to replace
> ne:CCC/ne:CCO when setting FLAGS_REG in neg patterns.
>
> gcc/
>
> PR target/107172
> * config/i386/i386.md (UNSPEC_CC_NE): New.
> Replace ne:CCC/ne:CCO with UNSPEC_CC_NE in neg patterns.
>
> gcc/testsuite/
>
> PR target/107172
> * gcc.target/i386/pr107172.c: New test.
Looking at the PR107172, comments #44 and #45, this patch is a trivial
substitution for an invalid RTX.
So, OK.
Thanks,
Uros.
> ---
> gcc/config/i386/i386.md | 45 +++++++++++++-----------
> gcc/testsuite/gcc.target/i386/pr107172.c | 26 ++++++++++++++
> 2 files changed, 51 insertions(+), 20 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr107172.c
>
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index baf1f1f8fa2..aaa678e7314 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -113,6 +113,7 @@ (define_c_enum "unspec" [
> UNSPEC_PEEPSIB
> UNSPEC_INSN_FALSE_DEP
> UNSPEC_SBB
> + UNSPEC_CC_NE
>
> ;; For SSE/MMX support:
> UNSPEC_FIX_NOTRUNC
> @@ -11470,7 +11471,7 @@ (define_insn_and_split "*neg<dwi>2_doubleword"
> "&& reload_completed"
> [(parallel
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC (match_dup 1) (const_int 0)))
> + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE))
> (set (match_dup 0) (neg:DWIH (match_dup 1)))])
> (parallel
> [(set (match_dup 2)
> @@ -11499,7 +11500,8 @@ (define_peephole2
> (match_operand:SWI48 1 "nonimmediate_gr_operand"))
> (parallel
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC (match_operand:SWI48 2 "general_reg_operand") (const_int 0)))
> + (unspec:CCC [(match_operand:SWI48 2 "general_reg_operand")
> + (const_int 0)] UNSPEC_CC_NE))
> (set (match_dup 2) (neg:SWI48 (match_dup 2)))])
> (parallel
> [(set (match_dup 0)
> @@ -11517,7 +11519,7 @@ (define_peephole2
> && !reg_mentioned_p (operands[2], operands[1])"
> [(parallel
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC (match_dup 2) (const_int 0)))
> + (unspec:CCC [(match_dup 2) (const_int 0)] UNSPEC_CC_NE))
> (set (match_dup 2) (neg:SWI48 (match_dup 2)))])
> (parallel
> [(set (match_dup 0)
> @@ -11543,7 +11545,8 @@ (define_peephole2
> (clobber (reg:CC FLAGS_REG))])
> (parallel
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC (match_operand:SWI48 1 "general_reg_operand") (const_int 0)))
> + (unspec:CCC [(match_operand:SWI48 1 "general_reg_operand")
> + (const_int 0)] UNSPEC_CC_NE))
> (set (match_dup 1) (neg:SWI48 (match_dup 1)))])
> (parallel
> [(set (match_dup 0)
> @@ -11559,7 +11562,7 @@ (define_peephole2
> "REGNO (operands[0]) != REGNO (operands[1])"
> [(parallel
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC (match_dup 1) (const_int 0)))
> + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE))
> (set (match_dup 1) (neg:SWI48 (match_dup 1)))])
> (parallel
> [(set (match_dup 0)
> @@ -11635,9 +11638,9 @@ (define_insn "*negsi_2_zext"
>
> (define_insn "*neg<mode>_ccc_1"
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC
> - (match_operand:SWI 1 "nonimmediate_operand" "0")
> - (const_int 0)))
> + (unspec:CCC
> + [(match_operand:SWI 1 "nonimmediate_operand" "0")
> + (const_int 0)] UNSPEC_CC_NE))
> (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m")
> (neg:SWI (match_dup 1)))]
> ""
> @@ -11647,9 +11650,9 @@ (define_insn "*neg<mode>_ccc_1"
>
> (define_insn "*neg<mode>_ccc_2"
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC
> - (match_operand:SWI 1 "nonimmediate_operand" "0")
> - (const_int 0)))
> + (unspec:CCC
> + [(match_operand:SWI 1 "nonimmediate_operand" "0")
> + (const_int 0)] UNSPEC_CC_NE))
> (clobber (match_scratch:SWI 0 "=<r>"))]
> ""
> "neg{<imodesuffix>}\t%0"
> @@ -11659,8 +11662,8 @@ (define_insn "*neg<mode>_ccc_2"
> (define_expand "x86_neg<mode>_ccc"
> [(parallel
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC (match_operand:SWI48 1 "register_operand")
> - (const_int 0)))
> + (unspec:CCC [(match_operand:SWI48 1 "register_operand")
> + (const_int 0)] UNSPEC_CC_NE))
> (set (match_operand:SWI48 0 "register_operand")
> (neg:SWI48 (match_dup 1)))])])
>
> @@ -11686,8 +11689,9 @@ (define_insn "*negqi_ext<mode>_2"
> ;; Negate with jump on overflow.
> (define_expand "negv<mode>3"
> [(parallel [(set (reg:CCO FLAGS_REG)
> - (ne:CCO (match_operand:SWI 1 "register_operand")
> - (match_dup 3)))
> + (unspec:CCO
> + [(match_operand:SWI 1 "register_operand")
> + (match_dup 3)] UNSPEC_CC_NE))
> (set (match_operand:SWI 0 "register_operand")
> (neg:SWI (match_dup 1)))])
> (set (pc) (if_then_else
> @@ -11703,8 +11707,9 @@ (define_expand "negv<mode>3"
>
> (define_insn "*negv<mode>3"
> [(set (reg:CCO FLAGS_REG)
> - (ne:CCO (match_operand:SWI 1 "nonimmediate_operand" "0")
> - (match_operand:SWI 2 "const_int_operand")))
> + (unspec:CCO [(match_operand:SWI 1 "nonimmediate_operand" "0")
> + (match_operand:SWI 2 "const_int_operand")]
> + UNSPEC_CC_NE))
> (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m")
> (neg:SWI (match_dup 1)))]
> "ix86_unary_operator_ok (NEG, <MODE>mode, operands)
> @@ -11770,7 +11775,7 @@ (define_insn_and_split "*abs<dwi>2_doubleword"
> "&& 1"
> [(parallel
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC (match_dup 1) (const_int 0)))
> + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE))
> (set (match_dup 2) (neg:DWIH (match_dup 1)))])
> (parallel
> [(set (match_dup 5)
> @@ -11814,7 +11819,7 @@ (define_insn_and_split "*nabs<dwi>2_doubleword"
> "&& 1"
> [(parallel
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC (match_dup 1) (const_int 0)))
> + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE))
> (set (match_dup 2) (neg:DWIH (match_dup 1)))])
> (parallel
> [(set (match_dup 5)
> @@ -21456,7 +21461,7 @@ (define_split
> (const_int 0))))]
> ""
> [(set (reg:CCC FLAGS_REG)
> - (ne:CCC (match_dup 1) (const_int 0)))
> + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE))
> (set (match_dup 0)
> (neg:SWI (ltu:SWI (reg:CCC FLAGS_REG) (const_int 0))))])
>
> diff --git a/gcc/testsuite/gcc.target/i386/pr107172.c b/gcc/testsuite/gcc.target/i386/pr107172.c
> new file mode 100644
> index 00000000000..d2c85f3f47c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr107172.c
> @@ -0,0 +1,26 @@
> +/* { dg-do run } */
> +/* { dg-options "-O1 -ftree-vrp" } */
> +
> +int a, c, d;
> +int
> +main()
> +{
> + long e = 1;
> + int f = a = 1;
> +L1:
> + if (a)
> + a = 2;
> + int h = e = ~e;
> + c = -1;
> + if (e >= a)
> + goto L2;
> + if (-1 > a)
> + goto L1;
> + if (a)
> + f = -1;
> +L2:
> + d = (-f + d) & h;
> + if (d)
> + __builtin_abort();
> + return 0;
> +}
> --
> 2.37.3
>
next prev parent reply other threads:[~2022-10-27 6:06 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-26 18:58 H.J. Lu
2022-10-27 6:06 ` Uros Bizjak [this message]
2022-10-28 5:56 ` Hongtao Liu
2022-10-28 6:08 ` Hongtao Liu
2022-10-28 8:35 ` Eric Botcazou
2022-10-28 15:51 ` H.J. Lu
2022-10-28 17:58 ` Eric Botcazou
2022-10-28 21:43 ` Segher Boessenkool
2022-10-28 21:55 ` Eric Botcazou
2022-11-01 20:34 ` Segher Boessenkool
2022-11-01 21:28 ` Eric Botcazou
2022-10-28 21:32 ` Segher Boessenkool
2022-10-28 22:28 ` H.J. Lu
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='CAFULd4ZzZ6S3dig8YhD4Soqx+nVspE=zsP5Q3tHv_6nDhuBEJg@mail.gmail.com' \
--to=ubizjak@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hjl.tools@gmail.com \
/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).