* [PATCH] x86: Fix cmov cost model issue [PR109549]
[not found] <20240506024925.1731048-1-lingling.kong@intel.com>
@ 2024-05-06 3:03 ` Kong, Lingling
2024-05-06 3:19 ` Hongtao Liu
0 siblings, 1 reply; 3+ messages in thread
From: Kong, Lingling @ 2024-05-06 3:03 UTC (permalink / raw)
To: gcc-patches; +Cc: Liu, Hongtao, Kong, Lingling
Hi,
(if_then_else:SI (eq (reg:CCZ 17 flags)
(const_int 0 [0]))
(reg/v:SI 101 [ e ])
(reg:SI 102))
The cost is 8 for the rtx, the cost for
(eq (reg:CCZ 17 flags) (const_int 0 [0])) is 4, but this is just an operator do not need to compute it's cost in cmov.
Bootstrapped and regtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/ChangeLog:
PR target/109549
* config/i386/i386.cc (ix86_rtx_costs): The XEXP (x, 0) for cmov
is an operator do not need to compute cost.
gcc/testsuite/ChangeLog:
* gcc.target/i386/cmov6.c: Fixed.
---
gcc/config/i386/i386.cc | 2 +-
gcc/testsuite/gcc.target/i386/cmov6.c | 5 +----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index 4d6b2b98761..59b4ce3bfbf 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -22237,7 +22237,7 @@ ix86_rtx_costs (rtx x, machine_mode mode, int outer_code_i, int opno,
{
/* cmov. */
*total = COSTS_N_INSNS (1);
- if (!REG_P (XEXP (x, 0)))
+ if (!COMPARISON_P (XEXP (x, 0)) && !REG_P (XEXP (x, 0)))
*total += rtx_cost (XEXP (x, 0), mode, code, 0, speed);
if (!REG_P (XEXP (x, 1)))
*total += rtx_cost (XEXP (x, 1), mode, code, 1, speed); diff --git a/gcc/testsuite/gcc.target/i386/cmov6.c b/gcc/testsuite/gcc.target/i386/cmov6.c
index 5111c8a9099..535326e4c2a 100644
--- a/gcc/testsuite/gcc.target/i386/cmov6.c
+++ b/gcc/testsuite/gcc.target/i386/cmov6.c
@@ -1,9 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2 -march=k8" } */
-/* if-converting this sequence would require two cmov
- instructions and seems to always cost more independent
- of the TUNE_ONE_IF_CONV setting. */
-/* { dg-final { scan-assembler-not "cmov\[^6\]" } } */
+/* { dg-final { scan-assembler "cmov\[^6\]" } } */
/* Verify that blocks are converted to conditional moves. */ extern int bar (int, int);
--
2.31.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86: Fix cmov cost model issue [PR109549]
2024-05-06 3:03 ` [PATCH] x86: Fix cmov cost model issue [PR109549] Kong, Lingling
@ 2024-05-06 3:19 ` Hongtao Liu
2024-05-06 6:06 ` Uros Bizjak
0 siblings, 1 reply; 3+ messages in thread
From: Hongtao Liu @ 2024-05-06 3:19 UTC (permalink / raw)
To: Kong, Lingling; +Cc: gcc-patches, Liu, Hongtao, Uros Bizjak
CC uros.
On Mon, May 6, 2024 at 11:03 AM Kong, Lingling <lingling.kong@intel.com> wrote:
>
> Hi,
> (if_then_else:SI (eq (reg:CCZ 17 flags)
> (const_int 0 [0]))
> (reg/v:SI 101 [ e ])
> (reg:SI 102))
> The cost is 8 for the rtx, the cost for
> (eq (reg:CCZ 17 flags) (const_int 0 [0])) is 4, but this is just an operator do not need to compute it's cost in cmov.
It looks like a reasonable change to me, for cmov, the first operand
of if_then_else is not a mask.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu.
> OK for trunk?
>
> gcc/ChangeLog:
>
> PR target/109549
> * config/i386/i386.cc (ix86_rtx_costs): The XEXP (x, 0) for cmov
> is an operator do not need to compute cost.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/cmov6.c: Fixed.
> ---
> gcc/config/i386/i386.cc | 2 +-
> gcc/testsuite/gcc.target/i386/cmov6.c | 5 +----
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index 4d6b2b98761..59b4ce3bfbf 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -22237,7 +22237,7 @@ ix86_rtx_costs (rtx x, machine_mode mode, int outer_code_i, int opno,
> {
> /* cmov. */
> *total = COSTS_N_INSNS (1);
> - if (!REG_P (XEXP (x, 0)))
> + if (!COMPARISON_P (XEXP (x, 0)) && !REG_P (XEXP (x, 0)))
> *total += rtx_cost (XEXP (x, 0), mode, code, 0, speed);
> if (!REG_P (XEXP (x, 1)))
> *total += rtx_cost (XEXP (x, 1), mode, code, 1, speed); diff --git a/gcc/testsuite/gcc.target/i386/cmov6.c b/gcc/testsuite/gcc.target/i386/cmov6.c
> index 5111c8a9099..535326e4c2a 100644
> --- a/gcc/testsuite/gcc.target/i386/cmov6.c
> +++ b/gcc/testsuite/gcc.target/i386/cmov6.c
> @@ -1,9 +1,6 @@
> /* { dg-do compile } */
> /* { dg-options "-O2 -march=k8" } */
> -/* if-converting this sequence would require two cmov
> - instructions and seems to always cost more independent
> - of the TUNE_ONE_IF_CONV setting. */
> -/* { dg-final { scan-assembler-not "cmov\[^6\]" } } */
> +/* { dg-final { scan-assembler "cmov\[^6\]" } } */
>
> /* Verify that blocks are converted to conditional moves. */ extern int bar (int, int);
> --
> 2.31.1
>
--
BR,
Hongtao
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86: Fix cmov cost model issue [PR109549]
2024-05-06 3:19 ` Hongtao Liu
@ 2024-05-06 6:06 ` Uros Bizjak
0 siblings, 0 replies; 3+ messages in thread
From: Uros Bizjak @ 2024-05-06 6:06 UTC (permalink / raw)
To: Hongtao Liu; +Cc: Kong, Lingling, gcc-patches, Liu, Hongtao
On Mon, May 6, 2024 at 5:20 AM Hongtao Liu <crazylht@gmail.com> wrote:
>
> CC uros.
>
> On Mon, May 6, 2024 at 11:03 AM Kong, Lingling <lingling.kong@intel.com> wrote:
> >
> > Hi,
> > (if_then_else:SI (eq (reg:CCZ 17 flags)
> > (const_int 0 [0]))
> > (reg/v:SI 101 [ e ])
> > (reg:SI 102))
> > The cost is 8 for the rtx, the cost for
> > (eq (reg:CCZ 17 flags) (const_int 0 [0])) is 4, but this is just an operator do not need to compute it's cost in cmov.
> It looks like a reasonable change to me, for cmov, the first operand
> of if_then_else is not a mask.
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu.
> > OK for trunk?
> >
> > gcc/ChangeLog:
> >
> > PR target/109549
> > * config/i386/i386.cc (ix86_rtx_costs): The XEXP (x, 0) for cmov
> > is an operator do not need to compute cost.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/i386/cmov6.c: Fixed.
OK.
BTW: I'd like to point out PR85559 [1] that collects some persistent
issues with x86 CMOV insn, especially [2].
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=cmov
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56309
Uros.
> > ---
> > gcc/config/i386/i386.cc | 2 +-
> > gcc/testsuite/gcc.target/i386/cmov6.c | 5 +----
> > 2 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index 4d6b2b98761..59b4ce3bfbf 100644
> > --- a/gcc/config/i386/i386.cc
> > +++ b/gcc/config/i386/i386.cc
> > @@ -22237,7 +22237,7 @@ ix86_rtx_costs (rtx x, machine_mode mode, int outer_code_i, int opno,
> > {
> > /* cmov. */
> > *total = COSTS_N_INSNS (1);
> > - if (!REG_P (XEXP (x, 0)))
> > + if (!COMPARISON_P (XEXP (x, 0)) && !REG_P (XEXP (x, 0)))
> > *total += rtx_cost (XEXP (x, 0), mode, code, 0, speed);
> > if (!REG_P (XEXP (x, 1)))
> > *total += rtx_cost (XEXP (x, 1), mode, code, 1, speed); diff --git a/gcc/testsuite/gcc.target/i386/cmov6.c b/gcc/testsuite/gcc.target/i386/cmov6.c
> > index 5111c8a9099..535326e4c2a 100644
> > --- a/gcc/testsuite/gcc.target/i386/cmov6.c
> > +++ b/gcc/testsuite/gcc.target/i386/cmov6.c
> > @@ -1,9 +1,6 @@
> > /* { dg-do compile } */
> > /* { dg-options "-O2 -march=k8" } */
> > -/* if-converting this sequence would require two cmov
> > - instructions and seems to always cost more independent
> > - of the TUNE_ONE_IF_CONV setting. */
> > -/* { dg-final { scan-assembler-not "cmov\[^6\]" } } */
> > +/* { dg-final { scan-assembler "cmov\[^6\]" } } */
> >
> > /* Verify that blocks are converted to conditional moves. */ extern int bar (int, int);
> > --
> > 2.31.1
> >
>
>
> --
> BR,
> Hongtao
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-06 6:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20240506024925.1731048-1-lingling.kong@intel.com>
2024-05-06 3:03 ` [PATCH] x86: Fix cmov cost model issue [PR109549] Kong, Lingling
2024-05-06 3:19 ` Hongtao Liu
2024-05-06 6:06 ` Uros Bizjak
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).