[Bug target/30961] New: redundant reg/mem stores/moves

[Bug target/30961] New: redundant reg/mem stores/moves

[pluto at agmk dot net]

float convert( unsigned in )
{
        float f;
        __builtin_memcpy( &f, &in, sizeof( in ) );
        return f;
}

unsigned short load( void* p )
{
        unsigned short v;
        __builtin_memcpy( &v, p, sizeof( v ) );
        return v;
}

gcc-4.2.0-20070221

$ g++ temp.cpp -O2 -S

_Z7convertj:
        movl    %edi, -4(%rsp)
        movss   -4(%rsp), %xmm1
        movaps  %xmm1, %xmm0
        ret

_Z4loadPv:
        movzwl  (%rdi), %eax
        movw    %ax, -2(%rsp)
        movzwl  %ax, %eax
        ret


-- 
           Summary: redundant reg/mem stores/moves
           Product: gcc
           Version: 4.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: pluto at agmk dot net
GCC target triplet: x86_64-linux


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

[Bug target/30961] [4.2 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #1 from pluto at agmk dot net  2007-03-05 11:11 -------
3.4.6 generates a nice code:

_Z7convertj:
        movl    %edi, -4(%rsp)
        movss   -4(%rsp), %xmm0
        ret

_Z4loadPv:
        movzwl  (%rdi), %eax
        ret


-- 

pluto at agmk dot net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.2.0
      Known to work|                            |3.4.6
            Summary|redundant reg/mem           |[4.2 regression] redundant
                   |stores/moves                |reg/mem stores/moves


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #2 from pluto at agmk dot net  2007-04-11 20:00 -------
4.2.0-RC1 has the same bug.


-- 


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[steven at gcc dot gnu dot org]

------- Comment #3 from steven at gcc dot gnu dot org  2007-04-11 22:40 -------
investigating...


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |steven at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-04-11 22:40:11
               date|                            |


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[steven at gcc dot gnu dot org]

------- Comment #4 from steven at gcc dot gnu dot org  2007-05-11 18:22 -------
For "convert" the issue is caused by a decision that reload makes, but I don't
understand why.  The .lreg dump is this:

;; Start of basic block 2, registers live: 5 [di] 6 [bp] 7 [sp] 16 [argp] 20
[frame]
;; Pred edge  ENTRY [100.0%]  (fallthru)
(note:HI 4 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)

(insn:HI 2 4 3 2 t.c:3 (set (reg/v:SI 59 [ in ])
        (reg:SI 5 di [ in ])) 40 {*movsi_1} (nil)
    (expr_list:REG_DEAD (reg:SI 5 di [ in ])
        (nil)))

(note:HI 3 2 6 2 NOTE_INSN_FUNCTION_BEG)

(insn:HI 6 3 10 2 t.c:3 (set (reg:SF 58 [ <result> ])
        (subreg:SF (reg/v:SI 59 [ in ]) 0)) 90 {*movsf_1}
(insn_list:REG_DEP_TRUE 2 (nil))
    (expr_list:REG_DEAD (reg/v:SI 59 [ in ])
        (nil)))

(insn:HI 10 6 16 2 t.c:7 (set (reg/i:SF 21 xmm0 [ <result> ])
        (reg:SF 58 [ <result> ])) 90 {*movsf_1} (insn_list:REG_DEP_TRUE 6
(nil))
    (expr_list:REG_DEAD (reg:SF 58 [ <result> ])
        (nil)))

(insn:HI 16 10 0 2 t.c:7 (use (reg/i:SF 21 xmm0 [ <result> ])) -1
(insn_list:REG_DEP_TRUE 10 (nil))
    (nil))
;; End of basic block 2, registers live: 6 [bp] 7 [sp] 16 [argp] 20 [frame] 21
[xmm0]
;; Succ edge  EXIT [100.0%]  (fallthru)




Then in .greg we reload things:

;; Function convert (convert)

;; 0 regs to allocate:
;; 58 conflicts: 58 7 21
;; 59 conflicts: 59 5 7

Spilling for insn 6.
Using reg 22 for reload 0
Spilling for insn 6.
Using reg 22 for reload 0

Reloads for insn # 6
Reload 0: reload_in (SF) = (reg:SF 5 di)
        SSE_REGS, RELOAD_FOR_INPUT (opnum = 1), can't combine
        reload_in_reg: (subreg:SF (reg/v:SI 5 di [orig:59 in ] [59]) 0)
        reload_reg_rtx: (reg:SF 22 xmm1)
;; Register dispositions:
58 in 21  59 in 5

;; Hard regs used:  5 21 22

(note:HI 1 0 4 NOTE_INSN_DELETED)

;; Start of basic block 2, registers live: 5 [di] 7 [sp]
;; Pred edge  ENTRY [100.0%]  (fallthru)
(note:HI 4 1 3 2 [bb 2] NOTE_INSN_BASIC_BLOCK)

(note:HI 3 4 22 2 NOTE_INSN_FUNCTION_BEG)

(insn 22 3 23 2 t.c:3 (set (mem/c:SF (plus:DI (reg/f:DI 7 sp)
                (const_int -4 [0xfffffffffffffffc])) [0 S4 A8])
        (reg:SF 5 di)) 90 {*movsf_1} (nil)
    (nil))

(insn 23 22 6 2 t.c:3 (set (reg:SF 22 xmm1)
        (mem/c:SF (plus:DI (reg/f:DI 7 sp)
                (const_int -4 [0xfffffffffffffffc])) [0 S4 A8])) 90 {*movsf_1}
(nil)
    (nil))

(insn:HI 6 23 16 2 t.c:3 (set (reg:SF 21 xmm0 [orig:58 <result> ] [58])
        (reg:SF 22 xmm1)) 90 {*movsf_1} (insn_list:REG_DEP_TRUE 2 (nil))
    (nil))

(insn 16 6 21 2 t.c:7 (use (reg/i:SF 21 xmm0 [ <result> ])) -1
(insn_list:REG_DEP_TRUE 10 (nil))
    (nil))
;; End of basic block 2, registers live: 6 [bp] 7 [sp] 16 [argp] 20 [frame] 21
[xmm0]
;; Succ edge  EXIT [100.0%]  (fallthru)

(note 21 16 0 NOTE_INSN_DELETED)


The only reason that I can think of for this problem, by looking at the dumps,
is the rather strange conflict of reg 58 with reg 21 (xmm0).  Reg 58 dies when
it is copied to xmm0 in insn 10, so we should tie the registers and coalesce. 
I'm not sure why we can't resolve this without the extra move.


-- 


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[steven at gcc dot gnu dot org]

------- Comment #5 from steven at gcc dot gnu dot org  2007-05-11 18:26 -------
For gcc 3.4, same function convert, lreg:


;; Register 60 in 5.
;; Register 62 in 0.
(note:HI 2 0 27 NOTE_INSN_DELETED)

;; Start of basic block 0, registers live: 5 [di] 6 [bp] 7 [sp] 16 [argp] 20
[frame]
(note:HI 27 2 3 0 [bb 0] NOTE_INSN_BASIC_BLOCK)

(insn:HI 3 27 4 0 (set (reg/v:SI 60 [ in ])
        (reg:SI 5 di [ in ])) 43 {*movsi_1_nointernunit} (nil)
    (expr_list:REG_DEAD (reg:SI 5 di [ in ])
        (nil)))

(note:HI 4 3 30 0 NOTE_INSN_FUNCTION_BEG)

(insn:HI 30 4 20 0 (set (subreg:SI (reg/v:SF 62 [ in ]) 0)
        (reg/v:SI 60 [ in ])) 43 {*movsi_1_nointernunit} (insn_list 3 (nil))
    (expr_list:REG_DEAD (reg/v:SI 60 [ in ])
        (nil)))

(note:HI 20 30 23 0 NOTE_INSN_FUNCTION_END)

(insn:HI 23 20 26 0 (set (reg/i:SF 21 xmm0 [ <result> ])
        (reg/v:SF 62 [ in ])) 91 {*movsf_1_nointerunit} (insn_list 30 (nil))
    (expr_list:REG_DEAD (reg/v:SF 62 [ in ])
        (nil)))

(insn:HI 26 23 0 0 (use (reg/i:SF 21 xmm0 [ <result> ])) -1 (insn_list 23
(nil))
    (nil))
;; End of basic block 0, registers live:
 6 [bp] 7 [sp] 16 [argp] 20 [frame] 21 [xmm0]



and later on .greg:


;; Function convert

;; 0 regs to allocate:
;; 60 conflicts: 60 5 7
;; 62 conflicts: 62 0 7

Spilling for insn 23.
Spilling for insn 23.

Reloads for insn # 23
Reload 0: reload_in (SF) = (reg/v:SF 0 ax [orig:62 in ] [62])
        SSE_REGS, RELOAD_FOR_INPUT (opnum = 1)
        reload_in_reg: (reg/v:SF 0 ax [orig:62 in ] [62])
        reload_reg_rtx: (reg/i:SF 21 xmm0 [ <result> ])
;; Register dispositions:
60 in 5  62 in 0

;; Hard regs used:  0 5 21

(note:HI 2 0 27 NOTE_INSN_DELETED)

;; Start of basic block 0, registers live: 5 [di] 7 [sp]
(note:HI 27 2 3 0 [bb 0] NOTE_INSN_BASIC_BLOCK)

(insn:HI 3 27 4 0 (set (reg/v:SI 5 di [orig:60 in ] [60])
        (reg:SI 5 di [ in ])) 43 {*movsi_1_nointernunit} (nil)
    (nil))

(note:HI 4 3 30 0 NOTE_INSN_FUNCTION_BEG)

(insn:HI 30 4 20 0 (set (reg:SI 0 ax [orig:62 in ] [62])
        (reg/v:SI 5 di [orig:60 in ] [60])) 43 {*movsi_1_nointernunit}
(insn_list 3 (nil))
    (nil))

(note:HI 20 30 33 0 NOTE_INSN_FUNCTION_END)

(insn 33 20 34 0 (set (mem:SF (plus:DI (reg/f:DI 7 sp)
                (const_int -4 [0xfffffffffffffffc])) [0 S4 A8])
        (reg/v:SF 0 ax [orig:62 in ] [62])) 91 {*movsf_1_nointerunit} (nil)
    (nil))

(insn 34 33 23 0 (set (reg/i:SF 21 xmm0 [ <result> ])
        (mem:SF (plus:DI (reg/f:DI 7 sp)
                (const_int -4 [0xfffffffffffffffc])) [0 S4 A8])) 91
{*movsf_1_nointerunit} (nil)
    (nil))

(insn:HI 23 34 26 0 (set (reg/i:SF 21 xmm0 [ <result> ])
        (reg/i:SF 21 xmm0 [ <result> ])) 91 {*movsf_1_nointerunit} (insn_list
30 (nil))
    (nil))

(insn 26 23 32 0 (use (reg/i:SF 21 xmm0 [ <result> ])) -1 (insn_list 23 (nil))
    (nil))
;; End of basic block 0, registers live:
 6 [bp] 7 [sp] 16 [argp] 20 [frame] 21 [xmm0]


-- 


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[steven at gcc dot gnu dot org]

------- Comment #6 from steven at gcc dot gnu dot org  2007-05-11 18:28 -------
Note that with SVN gcc 4.3 (20070506) lreg actually _does_ tie reg 58 and reg
21 in lreg:
;; Register 58 in 21.
;; Register 59 in 5.

But we then fail due to the confict, I think.


-- 


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[steven at gcc dot gnu dot org]

------- Comment #7 from steven at gcc dot gnu dot org  2007-05-11 21:42 -------
This bug is beyond my regalloc/reload fu, so unassigning...


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|steven at gcc dot gnu dot   |unassigned at gcc dot gnu
                   |org                         |dot org
             Status|ASSIGNED                    |NEW


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #8 from hjl at lucon dot org  2007-05-12 18:24 -------
The

unsigned short load( void* p )
{
        unsigned short v;
        __builtin_memcpy( &v, p, sizeof( v ) );
        return v;
}

regression was introduced by

http://gcc.gnu.org/ml/gcc-cvs/2005-07/msg00192.html


-- 

hjl at lucon dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fjahanian at apple dot com


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[steven at gcc dot gnu dot org]

------- Comment #9 from steven at gcc dot gnu dot org  2007-05-12 18:47 -------
I also looked at "load" but it is fixed on the dataflow branch, which will be
merged into the trunk soon (and thereby fixing this bug).


-- 


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #10 from hjl at lucon dot org  2007-05-12 19:03 -------
Gcc 4.3 gives the same output for

float convert1( unsigned in )
{
        float f;
        __builtin_memcpy( &f, &in, sizeof( in ) );
        return f;
}

float convert2( unsigned in )
{
  union
    {
      float f;
      unsigned i;
    } x;
  x.i = in;
  return x.f;
}

while gcc 3.4 can optimize the first one. At least, gcc 4.3 is consistent.


-- 


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #11 from hjl at lucon dot org  2007-05-12 22:27 -------
find_reloads gets

(insn:HI 6 3 10 2 x.i:2 (set (reg:SF 21 xmm0 [orig:58 <result> ] [58])
        (subreg:SF (reg/v:SI 5 di [orig:59 in ] [59]) 0)) 90 {*movsf_1}
(insn_list:REG_DEP_TRUE 2 (nil))
    (expr_list:REG_DEAD (reg/v:SI 5 di [orig:59 in ] [59])
        (nil)))

But this pattern is valid since inter-unit move isn't allowed. It then
tries

(insn:HI 6 3 10 2 x.i:2 (set (reg:SF 22 xmm1)
        (subreg:SF (reg/v:SI 5 di [orig:59 in ] [59]) 0)) 90 {*movsf_1}
(insn_list:REG_DEP_TRUE 2 (nil))
    (expr_list:REG_DEAD (reg/v:SI 5 di [orig:59 in ] [59])
        (nil)))

This pattern isn't allowed either for the same reason. But gen_reload
emits

(insn 22 3 23 2 x.i:2 (set (mem/c:SF (plus:DI (reg/f:DI 7 sp)
                (const_int -4 [0xfffffffffffffffc])) [0 S4 A8])
        (reg:SF 5 di)) 90 {*movsf_1} (nil)
    (nil))

(insn 23 22 6 2 x.i:2 (set (reg:SF 22 xmm1)
        (mem/c:SF (plus:DI (reg/f:DI 7 sp)
                (const_int -4 [0xfffffffffffffffc])) [0 S4 A8])) 90 {*movsf_1}
(
nil)
    (nil))


-- 


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #12 from hjl at lucon dot org  2007-05-13 06:36 -------
find_reg computes the cost of moving to target register too high. This kludge
adjusts the cost of moving to target register and seems to work for the
testcase:

--- gcc/reload1.c.sse   2007-04-27 05:46:12.000000000 -0700
+++ gcc/reload1.c       2007-05-12 22:30:58.000000000 -0700
@@ -1723,6 +1723,20 @@ find_reg (struct insn_chain *chain, int
   HARD_REG_SET not_usable;
   HARD_REG_SET used_by_other_reload;
   reg_set_iterator rsi;
+  rtx body = PATTERN (chain->insn);
+  unsigned int dest_reg;
+
+  if (GET_CODE (body) == SET)
+    {
+      rtx dest = SET_DEST (body);
+
+      if ((REG_P (dest)
+          || (GET_CODE (dest) == SUBREG
+              && REG_P (SUBREG_REG (dest)))))
+       dest_reg = reg_or_subregno (dest);
+    }
+  else
+    dest_reg = FIRST_PSEUDO_REGISTER;

   COPY_HARD_REG_SET (not_usable, bad_spill_regs);
   IOR_HARD_REG_SET (not_usable, bad_spill_regs_global);
@@ -1746,7 +1760,7 @@ find_reg (struct insn_chain *chain, int
          && ! TEST_HARD_REG_BIT (used_by_other_reload, regno)
          && HARD_REGNO_MODE_OK (regno, rl->mode))
        {
-         int this_cost = spill_cost[regno];
+         int this_cost = dest_reg == regno ? 0 : spill_cost[regno];
          int ok = 1;
          unsigned int this_nregs = hard_regno_nregs[regno][rl->mode];


-- 


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

[Bug target/30961] [4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #13 from hjl at lucon dot org  2007-05-13 20:08 -------
Created an attachment (id=13550)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13550&action=view)
An experimental patch

This patch works for the testcase. Testsuite results for ia32, Intel64 and ia64
are at

http://gcc.gnu.org/ml/gcc-testresults/2007-05/msg00644.html
http://gcc.gnu.org/ml/gcc-testresults/2007-05/msg00645.html
http://gcc.gnu.org/ml/gcc-testresults/2007-05/msg00646.html


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.2/4.3 regression]        |[4.1/4.2/4.3 regression]
                   |redundant reg/mem           |redundant reg/mem
                   |stores/moves                |stores/moves
   Target Milestone|---                         |4.1.3


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[mmitchel at gcc dot gnu dot org]

-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #14 from pluto at agmk dot net  2007-07-31 18:29 -------
(In reply to comment #13)
> Created an attachment (id=13550)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13550&action=view) [edit]
> An experimental patch
> 
> This patch works for the testcase.

i've applied this patch to 4.2.1 and compile testcases:

$ cat 421-pr30961.s32

convert:
        flds    4(%esp)
        ret

load:
        subl    $16, %esp
        movl    20(%esp), %eax
        movzwl  (%eax), %eax
        movw    %ax, 14(%esp)
        movzwl  %ax, %eax
        addl    $16, %esp
        ret

$ cat 421-pr30961.s64

convert:
        movl    %edi, -4(%rsp)
        movss   -4(%rsp), %xmm0
        ret

load:
        movzwl  (%rdi), %eax
        movw    %ax, -2(%rsp)
        movzwl  %ax, %eax
        ret

convert/m32/m64 look fine. load/m32/m64 still look unoptimal.


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #15 from hjl at lucon dot org  2007-07-31 19:35 -------
(In reply to comment #14)
> (In reply to comment #13)
> > Created an attachment (id=13550)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13550&action=view) [edit]
> > An experimental patch
> > 
> > This patch works for the testcase.
> 
> i've applied this patch to 4.2.1 and compile testcases:
> 
>
> 
> convert/m32/m64 look fine. load/m32/m64 still look unoptimal.
> 

Gcc 4.3 works:
        .text
        .p2align 4,,15
.globl convert
        .type   convert, @function
convert:
        movl    %edi, -4(%rsp)
        movss   -4(%rsp), %xmm0
        ret
        .size   convert, .-convert
        .p2align 4,,15
.globl load
        .type   load, @function
load:
        movzwl  (%rdi), %eax
        ret
        .size   load, .-load
        .ident  "GCC: (GNU) 4.3.0 20070731 (experimental) [trunk revision
127100]"


-- 


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

[Bug target/30961] [4.1/4.2 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #16 from pluto at agmk dot net  2007-08-01 11:30 -------
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > Created an attachment (id=13550)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13550&action=view) [edit]
> > > An experimental patch
> > > 
> > > This patch works for the testcase.
> > 
> > i've applied this patch to 4.2.1 and compile testcases:
> > 
> > convert/m32/m64 look fine. load/m32/m64 still look unoptimal.
> > 
> 
> Gcc 4.3 works:

naturally but its not even been released yet.
users awaiting regression fixes for released compilers ;)


-- 

pluto at agmk dot net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|4.0.2 4.1.2 4.2.0 4.3.0     |4.0.2 4.1.2 4.2.1
            Summary|[4.1/4.2/4.3 regression]    |[4.1/4.2 regression]
                   |redundant reg/mem           |redundant reg/mem
                   |stores/moves                |stores/moves


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

[Bug target/30961] [4.1/4.2 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #17 from hjl at lucon dot org  2007-08-02 14:20 -------
(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > (In reply to comment #13)
> > > > Created an attachment (id=13550)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13550&action=view) [edit]
> > > > An experimental patch
> > > > 
> > > > This patch works for the testcase.
> > > 
> > > i've applied this patch to 4.2.1 and compile testcases:
> > > 
> > > convert/m32/m64 look fine. load/m32/m64 still look unoptimal.
> > > 
> > 
> > Gcc 4.3 works:
> 
> naturally but its not even been released yet.
> users awaiting regression fixes for released compilers ;)
> 

Load is fixed by dataflow merge.


-- 


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

[Bug target/30961] [4.1/4.2 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #18 from pluto at agmk dot net  2007-08-06 17:49 -------
(In reply to comment #17)
> (In reply to comment #16)
> > (In reply to comment #15)
> > > (In reply to comment #14)
> > > > (In reply to comment #13)
> > > > > Created an attachment (id=13550)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13550&action=view) [edit]
> > > > > An experimental patch
> > > > > 
> > > > > This patch works for the testcase.
> > > > 
> > > > i've applied this patch to 4.2.1 and compile testcases:
> > > > 
> > > > convert/m32/m64 look fine. load/m32/m64 still look unoptimal.
> > > > 
> > > 
> > > Gcc 4.3 works:
> > 
> > naturally but its not even been released yet.
> > users awaiting regression fixes for released compilers ;)
> > 
> 
> Load is fixed by dataflow merge.
> 

Dear H.J.Lu,
i'm not asking about 4.3 because it's far away future
and dataflow merge won't be a part of next 4.2 release.
i understand that developers love to play with unstable tree
instead of backporting fixes but we're awaiting regression
fixes for released compilers. after 4.3 release you'll jump
to 4.4 and leave 4.3 regressions in the same way as you
left 4.2 regressions. so, how we can motivate the gcc team
to fix regressions in non-trunk tree? do you expect donations?


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #19 from hjl at lucon dot org  2007-08-06 18:02 -------
If we can find which patch causes this regression, it will help find a
solution for 4.2.


-- 

hjl at lucon dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|4.0.2 4.1.2 4.2.1           |4.0.2 4.1.2 4.2.1 4.3.0
            Summary|[4.1/4.2 regression]        |[4.1/4.2/4.3 regression]
                   |redundant reg/mem           |redundant reg/mem
                   |stores/moves                |stores/moves
            Version|4.2.0                       |4.3.0


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #20 from pluto at agmk dot net  2007-08-06 18:28 -------
(In reply to comment #19)
> If we can find which patch causes this regression, it will help find a
> solution for 4.2.
> 

i'll we do a bisect hunting...


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #21 from pluto at agmk dot net  2007-08-06 21:39 -------
rev. 101665:

convert:movl    %edi, -4(%rsp)
        movl    %edi, -20(%rsp)
        movss   -4(%rsp), %xmm0
        ret
load:   movzwl  (%rdi), %eax
        ret

rev. 101666 (-fforce-mem removal):

convert:movl    %edi, -4(%rsp)
        movl    %edi, -20(%rsp)
        movss   -4(%rsp), %xmm0
        ret
load:   movzwl  (%rdi), %eax
        movw    %ax, -2(%rsp)
        movzwl  %ax, %eax
        ret

rev. 116656 (patch for PR27567):

convert:movl    %edi, -4(%rsp)
        movss   -4(%rsp), %xmm1
        movaps  %xmm1, %xmm0
        ret
load:   movzwl  (%rdi), %eax
        movw    %ax, -2(%rsp)
        movzwl  %ax, %eax
        ret


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #22 from hjl at lucon dot org  2007-08-07 00:22 -------
There are 2 issues, convert and load. They may be caused by different
patches and should be tracked separately when doing binary search.


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #23 from pluto at agmk dot net  2007-08-07 07:22 -------
(In reply to comment #22)
> There are 2 issues, convert and load. They may be caused by different
> patches and should be tracked separately when doing binary search.

i've filled the 'load' issue as PR33008.
the gcc-reload-second-2.patch fixes 'convert' and finally this PR.


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #24 from hjl at lucon dot org  2007-08-07 12:57 -------
(In reply to comment #23)
> (In reply to comment #22)
> > There are 2 issues, convert and load. They may be caused by different
> > patches and should be tracked separately when doing binary search.
> 
> i've filled the 'load' issue as PR33008.
> the gcc-reload-second-2.patch fixes 'convert' and finally this PR.
> 

Which checkin caused the convert regression?


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #25 from pluto at agmk dot net  2007-08-07 17:51 -------
(In reply to comment #24)
> (In reply to comment #23)
> > (In reply to comment #22)
> > > There are 2 issues, convert and load. They may be caused by different
> > > patches and should be tracked separately when doing binary search.
> > 
> > i've filled the 'load' issue as PR33008.
> > the gcc-reload-second-2.patch fixes 'convert' and finally this PR.
> > 
> 
> Which checkin caused the convert regression?

i don't know but the 'convert' issue is almost fixed.
gcc-4.2 patched against PR27567 and gcc-reload-second-2.patch produces:

float convert_int2float( int in )
{
        float f;
        __builtin_memcpy( &f, &in, sizeof( f ) );
        return f;
}

double convert_long2double( long in )
{
        double f;
        __builtin_memcpy( &f, &in, sizeof( f ) );
        return f;
}

{-m64 -O2}

convert_int2float:
        movl    %edi, -4(%rsp)
        movss   -4(%rsp), %xmm0
        ret

convert_long2double:
        movq    %rdi, -8(%rsp)
        movsd   -8(%rsp), %xmm1  <== reload-second patch doesn't work here.
        movapd  %xmm1, %xmm0     <== /
        ret

{-m32 -O2 -fomit-frame-pointer}

convert_int2float:
        flds    4(%esp)
        ret

convert_long2double:
        fldl    4(%esp)
        ret


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #26 from pluto at agmk dot net  2007-08-07 17:53 -------
(In reply to comment #25)
errata:
> double convert_long2double( long in )
                              ^^^^ wrong post, should be 'long long'.


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #27 from hjl at lucon dot org  2007-08-07 18:10 -------
Created an attachment (id=14033)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14033&action=view)
An updated patch

My patch works on gcc 4.3:

[hjl@gnu-27 30961]$ cat c.c
double
convert4(long long in )
{
  double f;
  __builtin_memcpy( &f, &in, sizeof( in ) );
  return f;
}
[hjl@gnu-27 30961]$ make c.s
/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/  -S -O2
-fno-asynchronous-unwind-tables c.c
[hjl@gnu-27 30961]$ cat c.s
        .file   "c.c"
        .text
        .p2align 4,,15
.globl convert4
        .type   convert4, @function
convert4:
        movq    %rdi, -8(%rsp)
        movsd   -8(%rsp), %xmm0
        ret
        .size   convert4, .-convert4
        .ident  "GCC: (GNU) 4.3.0 20070806 (experimental) [trunk revision
127248]"
        .section        .note.GNU-stack,"",@progbits
[hjl@gnu-27 30961]$ 

If you can find which patch causes this regression, it will be easier to
find a solution for 4.2.


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #28 from pluto at agmk dot net  2007-08-07 21:20 -------
(In reply to comment #27)
> If you can find which patch causes this regression, it will be easier to
> find a solution for 4.2.

r80888 works fine (below) and r85770 is broken.
i can't bisect more dueto build errors.
gcc{42/40} rejects libbanshee/engine/setif-sort.c.

convert_int2float:
        movl    %edi, -4(%rsp)
        movss   -4(%rsp), %xmm0
        ret
convert_long2double:
        movq    %rdi, -8(%rsp)
        movlpd  -8(%rsp), %xmm0
        ret


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #29 from hjl at lucon dot org  2007-08-08 00:33 -------
With revision 85770, I got

[hjl@gnu-27 rrs]$ cat convert.s
        .file   "convert.c"
        .text
        .align 4
        .p2align 4,,15
.globl convert4
        .type   convert4, @function
convert4:
        movq    %rdi, -8(%rsp)
        movq    %rdi, -16(%rsp)
        movlpd  -8(%rsp), %xmm0
        ret
        .size   convert4, .-convert4
        .ident  "GCC: (GNU) 3.5.0 20040810 (experimental)"
        .section        .note.GNU-stack,"",@progbits

Remove the extra mvoq won't solve this regression.


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #30 from hjl at lucon dot org  2007-08-08 03:01 -------
This patch

http://gcc.gnu.org/ml/gcc-cvs/2006-09/msg00042.html

causes this regression.


-- 

hjl at lucon dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at redhat dot com


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[pluto at agmk dot net]

------- Comment #31 from pluto at agmk dot net  2007-08-08 21:50 -------
r84072 introduces an extra movq regression:

+2004-07-04  Richard Henderson  <rth@redhat.com>
+
+       * rtl.def (ADDRESSOF): Remove.
+       * alias.c (rtx_equal_for_memref_p): Don't handle addressof.
+       (find_base_term, memrefs_conflict_p): Likewise.
+       * cse.c (fixed_base_plus_p, find_best_addr: Likewise.
+       (fold_rtx, cse_insn, set_live_p): Likewise.
+       * dwarf2out.c (mem_loc_descriptor): Likewise.
+       (add_location_or_const_value_attribute): Likewise.
+       * emit-rtl.c (copy_insn_1): Likewise.
+       * explow.c (memory_address): Likewise.
+       * expmed.c (store_split_bit_field): Likewise.
+       * expr.c (expand_expr_real_1): Likewise.
+       * function.c (instantiate_decl
+       (instantiate_virtual_regs_1, fix_lexical_addr): Likewise.
+       * genrecog.c (preds, validate_pattern): Likewise.
+       * integrate.c (copy_rtx_and_substitute): Likewise.
+       * recog.c (general_operand, register_operand): Likewise.
+       (memory_address_p): Likwise.
+       * reload1.c (eliminate_regs, elimination_effects): Likewise.
+       * rtl.c (copy_rtx): Likewise.
+       * rtlanal.c (rtx_unstable_p, rtx_varies_p): Likewise.
+       (rtx_addr_can_trap_p, nonzero_address_p, address_cost): Likewise.
+       * rtlhooks.c (gen_lowpart_general): Likewise.
+       * stmt.c (expand_asm_operands): Likewise.
+       * web.c (entry_register, replace_ref, web_main): Likewise.
+       * config/alpha/alpha.c (input_operand, alpha_legitimate_address_p,
+       alpha_expand_block_move, alpha_expand_block_clear): Likewise.
+       * config/arm/arm.c (thumb_rtx_costs): Likewise.
+       * config/c4x/c4x.c (c4x_valid_operands): Likewise.
+       * config/frv/frv.c (move_destination_operand, move_source_operand,
+       condexec_dest_operand, condexec_source_operand,
+       condexec_memory_operand): Likewise.
+       * config/h8300/h8300.h (PREDICATE_CODES): Likewise.
+       * config/ia64/ia64.c (general_xfmode_operand): Likewise.
+       (destination_xfmode_operand): Likewise.
+       * config/mips/mips.h (PREDICATE_CODES): Likewise.
+       * config/mn10300/mn10300.c (mn10300_address_cost_1): Likewise.
+       * config/s390/s390.c (general_s_operand): Likewise.
+       * config/s390/s390.md (mov*): Likewise.
+       * config/sparc/sparc.h (PREDICATE_CODES): Likewise.
+
+       * c-typeck.c (c_mark_addressable): Don't put_var_into_stack.
+       * expr.c (expand_expr_real_1): Likewise.
+       * stmt.c (expand_decl): Likewise.
+       * config/c4x/c4x.c (c4x_expand_builtin): Likewise.
+
+       * function.c (struct fixup_replacement, struct insns_for_mem_entry,
+       postponed_insns, put_var_into_stack, put_reg_into_stack,
+       schedule_fixup_var_refs, fixup_var_refs, find_fixup_replacement,
+       fixup_var_refs_insns, fixup_var_refs_insns_with_hash,
+       fixup_var_refs_insn, fixup_var_refs_1, fixup_memory_subreg,
+       walk_fixup_memory_subreg, fixup_stack_1, optimize_bit_field,
+       gen_mem_addressof, flush_addressof, put_addressof_into_stack,
+       purge_bitfield_addressof_replacements, purge_addressof_replacements,
+       purge_addressof_1, insns_for_mem_hash, insns_for_mem_comp,
+       struct insns_for_mem_walk_info, insns_for_mem_walk,
+       compute_insns_for_mem, is_addressof, purge_addressof, setjmp_protect,
+       setjmp_protect_args): Remove.
+       (push_function_context_to): Don't handle var_refs_queue.
+       (pop_function_context_from, free_after_compilation): Likewise.
+       (instantiate_virtual_regs): Don't handle parm_reg_stack_loc.
+       (assign_parms, allocate_struct_function): Likewise.
+       (use_register_for_decl): New.
+       (expand_function_end): Don't setjmp_protect.
+       * function.h (struct emit_status): Update commentary.
+       (struct function): Remove x_max_parm_reg, x_parm_reg_stack_loc.
+       (max_parm_reg, parm_reg_stack_loc): Remove.
+       * passes.c (DFI_addressof): Remove.
+       (dump_file_info): Remove addressof.
+       (rest_of_handle_addressof): Remove.
+       (rest_of_compilation): Don't call it.
+       * rtl.h (ADDRESSOF_REGNO, ADDRESSOF_DECL): Remove.
+       * stmt.c (expand_decl): Use use_register_for_decl.
+       * tree.h: Update decls.
+       * web.c (mark_addressof): Remove.
+       * doc/invoke.texi (-dF): Remove.
+


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[jakub at gcc dot gnu dot org]

------- Comment #32 from jakub at gcc dot gnu dot org  2007-08-27 14:49 -------
What's the state of the http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30961#c27
patch?  I haven't seen it posted to gcc-patches...


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #33 from hjl at lucon dot org  2007-08-27 18:38 -------
A patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01865.html


-- 

hjl at lucon dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://gcc.gnu.org/ml/gcc-
                   |                            |patches/2007-
                   |                            |08/msg01865.html


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at lucon dot org]

------- Comment #34 from hjl at lucon dot org  2007-11-06 17:30 -------
An updated patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2007-11/msg00273.html


-- 


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

[Bug target/30961] [4.1/4.2/4.3 regression] redundant reg/mem stores/moves

[hjl at gcc dot gnu dot org]

------- Comment #35 from hjl at gcc dot gnu dot org  2007-11-06 19:19 -------
Subject: Bug 30961

Author: hjl
Date: Tue Nov  6 19:19:23 2007
New Revision: 129943

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129943
Log:
gcc/

2007-11-06  Ulrich Weigand  <Ulrich.Weigand@de.ibm.com>

        PR target/30961
        * reload1.c (find_reloads): Also check in_reg when handling a
        simple move with an input reload and a destination of a hard
        register.

gcc/testsuite/

2007-11-06  H.J. Lu  <hongjiu.lu@intel.com>

        PR target/30961
        * gcc.target/i386/pr30961-1.c: New.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr30961-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/reload.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug target/30961] [4.2 regression] redundant reg/mem stores/moves

[jsm28 at gcc dot gnu dot org]

------- Comment #36 from jsm28 at gcc dot gnu dot org  2008-07-04 21:59 -------
Closing 4.1 branch.


-- 

jsm28 at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.1/4.2 regression]        |[4.2 regression] redundant
                   |redundant reg/mem           |reg/mem stores/moves
                   |stores/moves                |
   Target Milestone|4.1.3                       |4.2.5


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

[Bug target/30961] [4.2 regression] redundant reg/mem stores/moves

[hjl dot tools at gmail dot com]

------- Comment #37 from hjl dot tools at gmail dot com  2009-02-13 16:00 -------
Gcc 4.4.0 revision 144128 generates:

.globl convert
        .type   convert, @function
convert:
        movl    %edi, -4(%rsp)
        movss   -4(%rsp), %xmm0
        ret
        .size   convert, .-convert
        .p2align 4,,15
.globl load
        .type   load, @function
load:
        movzwl  (%rdi), %eax
        ret
        .size   load, .-load


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|3.4.6 4.3.0                 |3.4.6 4.3.0 4.4.0


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

[Bug target/30961] [4.2 regression] redundant reg/mem stores/moves

[jsm28 at gcc dot gnu dot org]

------- Comment #38 from jsm28 at gcc dot gnu dot org  2009-03-30 21:39 -------
Closing 4.2 branch, fixed in 4.3.


-- 

jsm28 at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
      Known to fail|4.0.2 4.1.2 4.2.3           |4.0.2 4.1.2 4.2.3 4.2.5
         Resolution|                            |FIXED
   Target Milestone|4.2.5                       |4.3.0


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