| Message ID | BN9PR11MB5483878009BFC3EA7700736AEC1C2@BN9PR11MB5483.namprd11.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Fix cmov cost model issue [PR109549] | expand |
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 >
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
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
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(-)