diff mbox series

Fix minor SLSR pessimization

Message ID 2513721.BpYKv4EoCR@arcturus.home
State New
Headers show
Series Fix minor SLSR pessimization | expand

Commit Message

Eric Botcazou Aug. 2, 2019, 1:15 p.m. UTC
Hi,

an user reported that, for pairs of consecutive memory accesses, the SLSR pass 
can slightly pessimize the generated code at -O2 on the x86 architecture:

struct x
{
  int a[16];
  int b[16];
};

void
set (struct x *p, unsigned int n, int i)
{
  p->a[n] = i;
  p->b[n] = i;
}

is compiled with SLSR enabled into:

        leaq    (%rdi,%rsi,4), %rax
        movl    %edx, (%rax)
        movl    %edx, 64(%rax)

which is slightly worse than the expected:

        movl    %edx, (%rdi,%rsi,4)
        movl    %edx, 64(%rdi,%rsi,4)

The attached patch is a tentative fix which doesn't seem to break anything.

Tested on x86_64-suse-linux, OK for the mainline?


2019-08-02  Eric Botcazou  <ebotcazou@adacore.com>

	* gimple-ssa-strength-reduction.c (valid_mem_ref_cand_p): New function.
	(replace_ref): Do not replace a chain of only two candidates which are
	valid memory references.


2019-08-02  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.dg/tree-ssa/slsr-42.c: New test.

Comments

Bill Schmidt Aug. 2, 2019, 2:49 p.m. UTC | #1
On 8/2/19 8:15 AM, Eric Botcazou wrote:
> Hi,
>
> an user reported that, for pairs of consecutive memory accesses, the SLSR pass 
> can slightly pessimize the generated code at -O2 on the x86 architecture:
>
> struct x
> {
>   int a[16];
>   int b[16];
> };
>
> void
> set (struct x *p, unsigned int n, int i)
> {
>   p->a[n] = i;
>   p->b[n] = i;
> }
>
> is compiled with SLSR enabled into:
>
>         leaq    (%rdi,%rsi,4), %rax
>         movl    %edx, (%rax)
>         movl    %edx, 64(%rax)
>
> which is slightly worse than the expected:
>
>         movl    %edx, (%rdi,%rsi,4)
>         movl    %edx, 64(%rdi,%rsi,4)
>
> The attached patch is a tentative fix which doesn't seem to break anything.
>
> Tested on x86_64-suse-linux, OK for the mainline?

OK.  Thanks for the patch!

Bill
>
>
> 2019-08-02  Eric Botcazou  <ebotcazou@adacore.com>
>
> 	* gimple-ssa-strength-reduction.c (valid_mem_ref_cand_p): New function.
> 	(replace_ref): Do not replace a chain of only two candidates which are
> 	valid memory references.
>
>
> 2019-08-02  Eric Botcazou  <ebotcazou@adacore.com>
>
> 	* gcc.dg/tree-ssa/slsr-42.c: New test.
>
diff mbox series

Patch

Index: gimple-ssa-strength-reduction.c
===================================================================
--- gimple-ssa-strength-reduction.c	(revision 273907)
+++ gimple-ssa-strength-reduction.c	(working copy)
@@ -1999,6 +1999,23 @@  replace_ref (tree *expr, slsr_cand_t c)
   update_stmt (c->cand_stmt);
 }
 
+/* Return true if CAND_REF candidate C is a valid memory reference.  */
+
+static bool
+valid_mem_ref_cand_p (slsr_cand_t c)
+{
+  if (TREE_CODE (TREE_OPERAND (c->stride, 1)) != INTEGER_CST)
+    return false;
+
+  struct mem_address addr
+    = { NULL_TREE, c->base_expr, TREE_OPERAND (c->stride, 0),
+	TREE_OPERAND (c->stride, 1), wide_int_to_tree (sizetype, c->index) };
+
+  return
+    valid_mem_ref_p (TYPE_MODE (c->cand_type), TYPE_ADDR_SPACE (c->cand_type),
+		     &addr);
+}
+
 /* Replace CAND_REF candidate C, each sibling of candidate C, and each
    dependent of candidate C with an equivalent strength-reduced data
    reference.  */
@@ -2006,6 +2023,16 @@  replace_ref (tree *expr, slsr_cand_t c)
 static void
 replace_refs (slsr_cand_t c)
 {
+  /* Replacing a chain of only 2 candidates which are valid memory references
+     is generally counter-productive because you cannot recoup the additional
+     calculation added in front of them.  */
+  if (c->basis == 0
+      && c->dependent
+      && !lookup_cand (c->dependent)->dependent
+      && valid_mem_ref_cand_p (c)
+      && valid_mem_ref_cand_p (lookup_cand (c->dependent)))
+    return;
+
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fputs ("Replacing reference: ", dump_file);