public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Reduce cost of MEM (A + imm).
@ 2024-05-28  2:46 liuhongt
  2024-05-28  6:13 ` Uros Bizjak
  0 siblings, 1 reply; 4+ messages in thread
From: liuhongt @ 2024-05-28  2:46 UTC (permalink / raw)
  To: gcc-patches; +Cc: ubizjak

For MEM, rtx_cost iterates each subrtx, and adds up the costs,
so for MEM (reg) and MEM (reg + 4), the former costs 5,
the latter costs 9, it is not accurate for x86. Ideally
address_cost should be used, but it reduce cost too much.
So current solution is make constant disp as cheap as possible.

Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
Ok for trunk?

gcc/ChangeLog:

	PR target/67325
	* config/i386/i386.cc (ix86_rtx_costs): Reduce cost of MEM (A
	+ imm) to "cost of MEM (A)" + 1.

gcc/testsuite/ChangeLog:

	* gcc.target/i386/pr67325.c: New test.
---
 gcc/config/i386/i386.cc                 | 19 ++++++++++++++++++-
 gcc/testsuite/gcc.target/i386/pr67325.c |  7 +++++++
 2 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr67325.c

diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index 3e2a3a194f1..3936223bd20 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -22194,7 +22194,24 @@ ix86_rtx_costs (rtx x, machine_mode mode, int outer_code_i, int opno,
       /* An insn that accesses memory is slightly more expensive
          than one that does not.  */
       if (speed)
-        *total += 1;
+	{
+	  *total += 1;
+	  rtx addr = XEXP (x, 0);
+	  /* For MEM, rtx_cost iterates each subrtx, and adds up the costs,
+	     so for MEM (reg) and MEM (reg + 4), the former costs 5,
+	     the latter costs 9, it is not accurate for x86. Ideally
+	     address_cost should be used, but it reduce cost too much.
+	     So current solution is make constant disp as cheap as possible.  */
+	  if (GET_CODE (addr) == PLUS
+	      && CONST_INT_P (XEXP (addr, 1))
+	      && x86_64_immediate_operand (XEXP (addr, 1), Pmode))
+	    {
+	      *total += 1;
+	      *total += rtx_cost (XEXP (addr, 0), Pmode, PLUS, 0, speed);
+	      return true;
+	    }
+	}
+
       return false;
 
     case ZERO_EXTRACT:
diff --git a/gcc/testsuite/gcc.target/i386/pr67325.c b/gcc/testsuite/gcc.target/i386/pr67325.c
new file mode 100644
index 00000000000..c3c1e4c5b4d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr67325.c
@@ -0,0 +1,7 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-not "(?:sar|shr)" } } */
+
+int f(long*l){
+  return *l>>32;
+}
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Reduce cost of MEM (A + imm).
  2024-05-28  2:46 [PATCH] Reduce cost of MEM (A + imm) liuhongt
@ 2024-05-28  6:13 ` Uros Bizjak
  2024-05-28 10:45   ` [PATCH V2] " liuhongt
  0 siblings, 1 reply; 4+ messages in thread
From: Uros Bizjak @ 2024-05-28  6:13 UTC (permalink / raw)
  To: liuhongt; +Cc: gcc-patches

On Tue, May 28, 2024 at 4:48 AM liuhongt <hongtao.liu@intel.com> wrote:
>
> For MEM, rtx_cost iterates each subrtx, and adds up the costs,
> so for MEM (reg) and MEM (reg + 4), the former costs 5,
> the latter costs 9, it is not accurate for x86. Ideally
> address_cost should be used, but it reduce cost too much.
> So current solution is make constant disp as cheap as possible.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk?
>
> gcc/ChangeLog:
>
>         PR target/67325
>         * config/i386/i386.cc (ix86_rtx_costs): Reduce cost of MEM (A
>         + imm) to "cost of MEM (A)" + 1.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/i386/pr67325.c: New test.
> ---
>  gcc/config/i386/i386.cc                 | 19 ++++++++++++++++++-
>  gcc/testsuite/gcc.target/i386/pr67325.c |  7 +++++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr67325.c
>
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index 3e2a3a194f1..3936223bd20 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -22194,7 +22194,24 @@ ix86_rtx_costs (rtx x, machine_mode mode, int outer_code_i, int opno,
>        /* An insn that accesses memory is slightly more expensive
>           than one that does not.  */
>        if (speed)
> -        *total += 1;
> +       {
> +         *total += 1;
> +         rtx addr = XEXP (x, 0);
> +         /* For MEM, rtx_cost iterates each subrtx, and adds up the costs,
> +            so for MEM (reg) and MEM (reg + 4), the former costs 5,
> +            the latter costs 9, it is not accurate for x86. Ideally
> +            address_cost should be used, but it reduce cost too much.
> +            So current solution is make constant disp as cheap as possible.  */
> +         if (GET_CODE (addr) == PLUS
> +             && CONST_INT_P (XEXP (addr, 1))

IMO, there is no need for CONST_INT_P condition, we should also allow
symbol_ref, label_ref and const (all allowed by
x86_64_immediate_operand predicate), these all decay to an immediate
value.

Uros.

> +             && x86_64_immediate_operand (XEXP (addr, 1), Pmode))
> +           {
> +             *total += 1;
> +             *total += rtx_cost (XEXP (addr, 0), Pmode, PLUS, 0, speed);
> +             return true;
> +           }
> +       }
> +
>        return false;
>
>      case ZERO_EXTRACT:
> diff --git a/gcc/testsuite/gcc.target/i386/pr67325.c b/gcc/testsuite/gcc.target/i386/pr67325.c
> new file mode 100644
> index 00000000000..c3c1e4c5b4d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr67325.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile { target { ! ia32 } } } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { scan-assembler-not "(?:sar|shr)" } } */
> +
> +int f(long*l){
> +  return *l>>32;
> +}
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH V2] Reduce cost of MEM (A + imm).
  2024-05-28  6:13 ` Uros Bizjak
@ 2024-05-28 10:45   ` liuhongt
  2024-05-28 12:04     ` Uros Bizjak
  0 siblings, 1 reply; 4+ messages in thread
From: liuhongt @ 2024-05-28 10:45 UTC (permalink / raw)
  To: gcc-patches; +Cc: ubizjak

> IMO, there is no need for CONST_INT_P condition, we should also allow
> symbol_ref, label_ref and const (all allowed by
> x86_64_immediate_operand predicate), these all decay to an immediate
> value.

Changed.

Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
Ok for trunk.

For MEM, rtx_cost iterates each subrtx, and adds up the costs,
so for MEM (reg) and MEM (reg + 4), the former costs 5,
the latter costs 9, it is not accurate for x86. Ideally
address_cost should be used, but it reduce cost too much.
So current solution is make constant disp as cheap as possible.

gcc/ChangeLog:

	PR target/67325
	* config/i386/i386.cc (ix86_rtx_costs): Reduce cost of MEM (A
	+ imm) to "cost of MEM (A)" + 1.

gcc/testsuite/ChangeLog:

	* gcc.target/i386/pr67325.c: New test.
---
 gcc/config/i386/i386.cc                 | 18 +++++++++++++++++-
 gcc/testsuite/gcc.target/i386/pr67325.c |  7 +++++++
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr67325.c

diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index 3e2a3a194f1..85d87b9f778 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -22194,7 +22194,23 @@ ix86_rtx_costs (rtx x, machine_mode mode, int outer_code_i, int opno,
       /* An insn that accesses memory is slightly more expensive
          than one that does not.  */
       if (speed)
-        *total += 1;
+	{
+	  *total += 1;
+	  rtx addr = XEXP (x, 0);
+	  /* For MEM, rtx_cost iterates each subrtx, and adds up the costs,
+	     so for MEM (reg) and MEM (reg + 4), the former costs 5,
+	     the latter costs 9, it is not accurate for x86. Ideally
+	     address_cost should be used, but it reduce cost too much.
+	     So current solution is make constant disp as cheap as possible.  */
+	  if (GET_CODE (addr) == PLUS
+	      && x86_64_immediate_operand (XEXP (addr, 1), Pmode))
+	    {
+	      *total += 1;
+	      *total += rtx_cost (XEXP (addr, 0), Pmode, PLUS, 0, speed);
+	      return true;
+	    }
+	}
+
       return false;
 
     case ZERO_EXTRACT:
diff --git a/gcc/testsuite/gcc.target/i386/pr67325.c b/gcc/testsuite/gcc.target/i386/pr67325.c
new file mode 100644
index 00000000000..c3c1e4c5b4d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr67325.c
@@ -0,0 +1,7 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-not "(?:sar|shr)" } } */
+
+int f(long*l){
+  return *l>>32;
+}
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH V2] Reduce cost of MEM (A + imm).
  2024-05-28 10:45   ` [PATCH V2] " liuhongt
@ 2024-05-28 12:04     ` Uros Bizjak
  0 siblings, 0 replies; 4+ messages in thread
From: Uros Bizjak @ 2024-05-28 12:04 UTC (permalink / raw)
  To: liuhongt; +Cc: gcc-patches

On Tue, May 28, 2024 at 12:48 PM liuhongt <hongtao.liu@intel.com> wrote:
>
> > IMO, there is no need for CONST_INT_P condition, we should also allow
> > symbol_ref, label_ref and const (all allowed by
> > x86_64_immediate_operand predicate), these all decay to an immediate
> > value.
>
> Changed.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk.
>
> For MEM, rtx_cost iterates each subrtx, and adds up the costs,
> so for MEM (reg) and MEM (reg + 4), the former costs 5,
> the latter costs 9, it is not accurate for x86. Ideally
> address_cost should be used, but it reduce cost too much.
> So current solution is make constant disp as cheap as possible.
>
> gcc/ChangeLog:
>
>         PR target/67325
>         * config/i386/i386.cc (ix86_rtx_costs): Reduce cost of MEM (A
>         + imm) to "cost of MEM (A)" + 1.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/i386/pr67325.c: New test.

OK.

Thanks,
Uros.

> ---
>  gcc/config/i386/i386.cc                 | 18 +++++++++++++++++-
>  gcc/testsuite/gcc.target/i386/pr67325.c |  7 +++++++
>  2 files changed, 24 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr67325.c
>
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index 3e2a3a194f1..85d87b9f778 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -22194,7 +22194,23 @@ ix86_rtx_costs (rtx x, machine_mode mode, int outer_code_i, int opno,
>        /* An insn that accesses memory is slightly more expensive
>           than one that does not.  */
>        if (speed)
> -        *total += 1;
> +       {
> +         *total += 1;
> +         rtx addr = XEXP (x, 0);
> +         /* For MEM, rtx_cost iterates each subrtx, and adds up the costs,
> +            so for MEM (reg) and MEM (reg + 4), the former costs 5,
> +            the latter costs 9, it is not accurate for x86. Ideally
> +            address_cost should be used, but it reduce cost too much.
> +            So current solution is make constant disp as cheap as possible.  */
> +         if (GET_CODE (addr) == PLUS
> +             && x86_64_immediate_operand (XEXP (addr, 1), Pmode))
> +           {
> +             *total += 1;
> +             *total += rtx_cost (XEXP (addr, 0), Pmode, PLUS, 0, speed);
> +             return true;
> +           }
> +       }
> +
>        return false;
>
>      case ZERO_EXTRACT:
> diff --git a/gcc/testsuite/gcc.target/i386/pr67325.c b/gcc/testsuite/gcc.target/i386/pr67325.c
> new file mode 100644
> index 00000000000..c3c1e4c5b4d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr67325.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile { target { ! ia32 } } } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { scan-assembler-not "(?:sar|shr)" } } */
> +
> +int f(long*l){
> +  return *l>>32;
> +}
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-05-28 12:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-28  2:46 [PATCH] Reduce cost of MEM (A + imm) liuhongt
2024-05-28  6:13 ` Uros Bizjak
2024-05-28 10:45   ` [PATCH V2] " liuhongt
2024-05-28 12:04     ` 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).