[Bug target/50603] New: [x32] Unnecessary lea

[Bug target/50603] New: [x32] Unnecessary lea

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50603

             Bug #: 50603
           Summary: [x32] Unnecessary lea
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hjl.tools@gmail.com
                CC: ubizjak@gmail.com


[hjl@gnu-6 ilp32-2]$ cat int.c
extern int *foo;

int bar (int x) { return foo[x]; }
[hjl@gnu-6 ilp32-2]$ make int.s
/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/ -S -mx32  -O2 int.c
[hjl@gnu-6 ilp32-2]$ cat int.s
    .file    "int.c"
    .text
    .p2align 4,,15
    .globl    bar
    .type    bar, @function
bar:
.LFB0:
    .cfi_startproc
    movl    foo(%rip), %eax
    leal    (%rax,%rdi,4), %edi
    movl    (%edi), %eax
    ret
    .cfi_endproc
.LFE0:
    .size    bar, .-bar
    .ident    "GCC: (GNU) 4.7.0 20110923 (experimental)"
    .section    .note.GNU-stack,"",@progbits
[hjl@gnu-6 ilp32-2]$ 

We can generate

    movl    foo(%rip), %eax
    movl    (%eax,%edi,4), %eax
        ret

[Bug target/50603] [x32] Unnecessary lea

[ubizjak at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50603

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-10-03
     Ever Confirmed|0                           |1

--- Comment #1 from Uros Bizjak <ubizjak at gmail dot com> 2011-10-03 19:13:02 UTC ---
This is the same issue as in:

FAIL: gcc.target/i386/pr45670.c scan-assembler-not lea[lq]

We expand to addsi_1 with memory operand:

(insn 7 6 9 2 (parallel [
            (set (reg:SI 68)
                (plus:SI (reg:SI 69)
                    (mem/f/c/i:SI (symbol_ref:DI ("foo") [flags 0x40] 
<var_decl  0x7f36bc52a140 foo>) [2 foo+0 S4 A32])))
            (clobber (reg:CC 17 flags))
        ]) pr50603.c:3 253 {*addsi_1}
     (expr_list:REG_DEAD (reg:SI 69)
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

and then fail at combine with:

Trying 6, 7 -> 9:
Failed to match this instruction:
(set (reg:SI 70 [ *D.3235_5 ])
    (mem:SI (zero_extend:DI (plus:SI (mult:SI (reg/v:SI 65 [ x ])
                    (const_int 4 [0x4]))
                (mem/f/c/i:SI (symbol_ref:DI ("foo") [flags 0x40]  <var_decl
0x7f36bc52a140 foo>) [2 foo+0 S4 A32]))) [3 *D.3235_5+0 S4 A32]))

[Bug target/50603] [x32] Unnecessary lea

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50603

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |UNCONFIRMED
     Ever Confirmed|1                           |0

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> 2011-10-03 20:57:53 UTC ---
I am testing this patch:

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 7e89dbd..612fcd2 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -15713,6 +15713,16 @@ ix86_fixup_binary_operands (enum rtx_code code, enum
ma
chine_mode mode,
       else
     src2 = force_reg (mode, src2);
     }
+  else
+    {
+      /* Improve address combine in x32 mode.  */
+      if (TARGET_X32
+      && code == PLUS
+      && !MEM_P (dst)
+      && !MEM_P (src1)
+      && MEM_P (src2) )
+    src2 = force_reg (mode, src2);
+    }

   /* If the destination is memory, and we do not have matching source
      operands, do things in registers.  */

[Bug target/50603] [x32] Unnecessary lea

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50603

--- Comment #3 from H.J. Lu <hjl.tools at gmail dot com> 2011-10-06 19:00:54 UTC ---
[hjl@gnu-mic-2 pr50633]$ cat x.i
struct s { int val[16]; };

extern double f (struct s pb, double pc);

int main ()
{
  struct s x;
  int i;

  for (i = 0; i < 16; i++)
    x.val[i] = i + 1;
  if (f (x, 10000.0L) != 10136.0L)
    __builtin_abort ();
  return 0;
}
[hjl@gnu-mic-2 pr50633]$ make x.s
/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/ -mx32 -O -S x.i
[hjl@gnu-mic-2 pr50633]$ cat x.s
    .file    "x.i"
    .text
    .globl    main
    .type    main, @function
main:
.LFB0:
    .cfi_startproc
    subq    $136, %rsp
    .cfi_def_cfa_offset 144
    movl    $0, %eax
    movl    %esp, %ecx
    addl    $60, %ecx
.L2:
    addl    $1, %eax
    leal    (%rcx,%rax,4), %edx
    movl    %eax, (%edx)
    cmpl    $16, %eax
    jne    .L2
    movq    64(%rsp), %rax
    movq    %rax, (%rsp)
    movq    72(%rsp), %rax
    movq    %rax, 8(%rsp)
    movq    80(%rsp), %rax
    movq    %rax, 16(%rsp)
    movq    88(%rsp), %rax
    movq    %rax, 24(%rsp)
    movq    96(%rsp), %rax
    movq    %rax, 32(%rsp)
    movq    104(%rsp), %rax
    movq    %rax, 40(%rsp)
    movq    112(%rsp), %rax
    movq    %rax, 48(%rsp)
    movq    120(%rsp), %rax
    movq    %rax, 56(%rsp)
    movsd    .LC0(%rip), %xmm0
    call    f
    ucomisd    .LC1(%rip), %xmm0
    jp    .L5
    je    .L7
.L5:
    call    abort
.L7:
    movl    $0, %eax
    addq    $136, %rsp
    .cfi_def_cfa_offset 8
    ret
    .cfi_endproc
.LFE0:
    .size    main, .-main

    leal    (%rcx,%rax,4), %edx
    movl    %eax, (%edx)

can be combined into

       movl    %eax, (%ecx,%eax,4)

[Bug target/50603] [x32] Unnecessary lea

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50603

--- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> 2011-10-06 19:19:23 UTC ---
Combine failed:

(set (mem:SI (and:DI (plus:DI (subreg:DI (mult:SI (reg/v:SI 84 [ i ])
                        (const_int 4 [0x4])) 0)
                (subreg:DI (reg:SI 106) 0)) 
            (const_int 4294967292 [0xfffffffc])) [3 MEM[symbol: x, index:
D.2741_12, step: 4, offset: 4294967292B]+0 S4 A32])
    (reg/v:SI 84 [ i ])) 

for

(insn 37 35 39 3 (set (reg:SI 90)
        (plus:SI (mult:SI (reg/v:SI 84 [ i ])
                (const_int 4 [0x4]))
            (reg:SI 106))) x.i:11 247 {*leasi_2}
     (nil))

(insn 39 37 41 3 (set (mem:SI (zero_extend:DI (reg:SI 90)) [3 MEM[symbol: x,
index: D.2741_12, step: 4, offset: 4294967292B]+0 S4 A32])
        (reg/v:SI 84 [ i ])) x.i:11 64 {*movsi_internal}
     (expr_list:REG_DEAD (reg:SI 90)
        (nil)))

Since address is 32bit aligned, 0xfffffffc is the same as
0xffffffff.  But we don't have this information.

[Bug target/50603] [x32] Unnecessary lea

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50603

--- Comment #5 from H.J. Lu <hjl.tools at gmail dot com> 2011-10-06 19:37:56 UTC ---
Taking from combine.c:

     else if (GET_CODE (t) == ZERO_EXTEND
               && (GET_CODE (XEXP (t, 0)) == PLUS
                   || GET_CODE (XEXP (t, 0)) == IOR
                   || GET_CODE (XEXP (t, 0)) == XOR)
               && GET_CODE (XEXP (XEXP (t, 0), 1)) == SUBREG
               && HWI_COMPUTABLE_MODE_P (mode)
               && subreg_lowpart_p (XEXP (XEXP (t, 0), 1))
               && rtx_equal_p (SUBREG_REG (XEXP (XEXP (t, 0), 1)), f)
               && ((nonzero_bits (f, GET_MODE (f))
                    & ~GET_MODE_MASK (GET_MODE (XEXP (XEXP (t, 0), 1)))) 
                   == 0))
        {     
          c1 = XEXP (XEXP (t, 0), 0); z = f; op = GET_CODE (XEXP (t, 0));
          extend_op = ZERO_EXTEND;
          m = GET_MODE (XEXP (t, 0));
        }     

I think this patch:

diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index 89cc8a7..4386b82 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -600,8 +600,8 @@
 ;; Match exactly 0x0FFFFFFFF in anddi as a zero-extension operation
 (define_predicate "const_32bit_mask"
   (and (match_code "const_int")
-       (match_test "trunc_int_for_mode (INTVAL (op), DImode)
-            == (HOST_WIDE_INT) 0xffffffff")))
+       (match_test "(nonzero_bits (op, GET_MODE (op))
+             & ~GET_MODE_MASK (DImode)) == 0")))

 ;; Match 2, 4, or 8.  Used for leal multiplicands.
 (define_predicate "const248_operand"

should work.

[Bug target/50603] [x32] Unnecessary lea

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50603

--- Comment #6 from H.J. Lu <hjl.tools at gmail dot com> 2011-10-06 19:44:06 UTC ---
It doesn't work since we fail to decompose subreg.

[Bug target/50603] [x32] Unnecessary lea

[uros at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50603

--- Comment #7 from uros at gcc dot gnu.org 2011-10-07 06:43:24 UTC ---
Author: uros
Date: Fri Oct  7 06:43:17 2011
New Revision: 179646

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=179646
Log:
    PR target/50603
    * config/i386/i386.c (ix86_fixup_binary_operands): Force src2 of
    integer PLUS RTX to a register to improve address combine.

testsuite/ChangeLog:

    PR target/50603
    * gcc.target/i386/pr50603.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr50603.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/testsuite/ChangeLog

[Bug target/50603] [x32] Unnecessary lea

[ubizjak at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50603

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |x32
             Status|UNCONFIRMED                 |RESOLVED
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2011-10/msg00547.htm
                   |                            |l
         Resolution|                            |FIXED
   Target Milestone|---                         |4.7.0

--- Comment #8 from Uros Bizjak <ubizjak at gmail dot com> 2011-10-09 09:30:09 UTC ---
(In reply to comment #6)
> It doesn't work since we fail to decompose subreg.

The question here is, why combine creates:

Failed to match this instruction:
(set (mem:SI (and:DI (plus:DI (subreg:DI (mult:SI (reg/v:SI 85 [ i ])
                        (const_int 4 [0x4])) 0)
                (subreg:DI (reg:SI 106) 0))
            (const_int 4294967292 [0xfffffffc])) [0 MEM[symbol: x, index:
D.2741_1, step: 4, offset: 4294967292B]+0 S4 A32])
    (reg/v:SI 85 [ i ]))

Considering that this is in fact zero-extension, the "optimized" pattern is
worse than sticking subreg to the whole address, i.e.

(and:DI (subreg:DI (plus:SI (mult:SI (reg/v:SI 85 [ i ]) (const_int 4 [0x4]))
                            (reg:SI 106)) 0)
        (const_int 4294967295 [0xffffffff]))

Please note that we have registers in two different modes in the former
pattern. The later pattern would be recognized by i386.c code.

Anyway, the original problem is solved, instead of hijacking exisiting PR,
please open a new PR (IMO, RTL optimization).