[Bug rtl-optimization/23102] New: extra XORs generated on i686

[Bug rtl-optimization/23102] New: extra XORs generated on i686

[dann at godzilla dot ics dot uci dot edu]

Compiling the code below (extracted from xterm-202) with
 -fno-inline -O2 -march=i686

typedef unsigned int Cardinal;
typedef unsigned long Pixel;
typedef char *String;

typedef struct {
 String resource;
 Pixel value;
 int mode;
} ColorRes;

typedef struct {
 ColorRes Acolors[(256 +4)];
 int startHRow, startHCol,
   endHRow, endHCol,
   startHCoord, endHCoord;
 Cardinal selection_count;
} TScreen;

static void
ResetSelectionState(TScreen * screen)
{
    screen->selection_count = 0;
    screen->startHRow = screen->startHCol = 0;
    screen->endHRow = screen->endHCol = 0;
}

void foo (TScreen *scr)
{
  ResetSelectionState (scr);
}

generates: 
ResetSelectionState:
        pushl   %ebp
        xorl    %edx, %edx
        movl    %esp, %ebp
        xorl    %ecx, %ecx
        popl    %ebp
        movl    %edx, 3144(%eax)
        xorl    %edx, %edx
        movl    %ecx, 3124(%eax)
        xorl    %ecx, %ecx   ;; this is not needed, ecx is already 0 
        movl    %edx, 3120(%eax)
        xorl    %edx, %edx    ;; so is edx
        movl    %ecx, 3132(%eax)  
        movl    %edx, 3128(%eax)
        ret

when using -march=i386 the code looks better:

ResetSelectionState:
        pushl   %ebp
        movl    %esp, %ebp
        movl    $0, 3144(%eax)
        movl    $0, 3124(%eax)
        movl    $0, 3120(%eax)
        movl    $0, 3132(%eax)
        movl    $0, 3128(%eax)
        leave
        ret

-- 
           Summary: extra XORs  generated on i686
           Product: gcc
           Version: 4.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: rtl-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: dann at godzilla dot ics dot uci dot edu
                CC: gcc-bugs at gcc dot gnu dot org
  GCC host triplet: i686-pc-linux-gnu


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

[Bug target/23102] extra XORs generated on i686

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-07-27 22:18 -------
Confirmed, this is a i686 only problem as the complex movl are slower and we expand the movl after 
reload.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
             Status|UNCONFIRMED                 |NEW
          Component|rtl-optimization            |target
     Ever Confirmed|                            |1
   GCC host triplet|i686-pc-linux-gnu           |
 GCC target triplet|                            |i686-pc-linux-gnu
           Keywords|                            |missed-optimization
   Last reconfirmed|0000-00-00 00:00:00         |2005-07-27 22:18:58
               date|                            |


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

[Bug target/23102] extra XORs generated on i686

[dann at godzilla dot ics dot uci dot edu]

------- Additional Comments From dann at godzilla dot ics dot uci dot edu  2005-08-03 18:21 -------
These insns in the .32.flow2 RTL dump: 
(insn:HI 12 7 14 0 (set (mem/s:SI (plus:SI (reg/v/f:SI 0 ax [orig:58 screen ] [58])
                (const_int 3144 [0xc48])) [7 <variable>.selection_count+0 S4 A32])
        (const_int 0 [0x0])) 34 {*movsi_1} (insn_list:REG_DEP_TRUE 6 (nil))
    (nil))

(insn:HI 14 12 15 0 (set (mem/s:SI (plus:SI (reg/v/f:SI 0 ax [orig:58 screen ] [58])
                (const_int 3124 [0xc34])) [7 <variable>.startHCol+0 S4 A32])
        (const_int 0 [0x0])) 34 {*movsi_1} (nil)
    (nil))

(insn:HI 15 14 17 0 (set (mem/s:SI (plus:SI (reg/v/f:SI 0 ax [orig:58 screen ] [58])
                (const_int 3120 [0xc30])) [7 <variable>.startHRow+0 S4 A32])
        (const_int 0 [0x0])) 34 {*movsi_1} (nil)
    (nil))
[snip]
are transformed by this peephole2: 
(define_peephole2
  [(match_scratch:SI 1 "r")
   (set (match_operand:SI 0 "memory_operand" "")
        (const_int 0))]
  "! optimize_size
   && ! TARGET_USE_MOV0
   && TARGET_SPLIT_LONG_MOVES
   && get_attr_length (insn) >= ix86_cost->large_insn
   && peep2_regno_dead_p (0, FLAGS_REG)"
  [(parallel [(set (match_dup 1) (const_int 0))
	      (clobber (reg:CC FLAGS_REG))])
   (set (match_dup 0) (match_dup 1))]
  "")

into: 

(insn 46 45 43 0 (set (mem/s:SI (plus:SI (reg/v/f:SI 0 ax [orig:58 screen ] [58])
                (const_int 3144 [0xc48])) [7 <variable>.selection_count+0 S4 A32])
        (reg:SI 1 dx)) -1 (nil)
    (expr_list:REG_DEAD (reg:SI 1 dx)
        (nil)))

(insn 43 46 44 0 (parallel [
            (set (reg:SI 2 cx)
                (const_int 0 [0x0]))
            (clobber (reg:CC 17 flags))
        ]) -1 (nil)
    (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))

(insn 44 43 41 0 (set (mem/s:SI (plus:SI (reg/v/f:SI 0 ax [orig:58 screen ] [58])
                (const_int 3124 [0xc34])) [7 <variable>.startHCol+0 S4 A32])
        (reg:SI 2 cx)) -1 (nil)
    (expr_list:REG_DEAD (reg:SI 2 cx)
        (nil)))

(insn 41 44 42 0 (parallel [
            (set (reg:SI 1 dx)
                (const_int 0 [0x0]))
            (clobber (reg:CC 17 flags))
        ]) -1 (nil)
    (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))

(insn 42 41 39 0 (set (mem/s:SI (plus:SI (reg/v/f:SI 0 ax [orig:58 screen ] [58])
                (const_int 3120 [0xc30])) [7 <variable>.startHRow+0 S4 A32])
        (reg:SI 1 dx)) -1 (nil)
    (expr_list:REG_DEAD (reg:SI 1 dx)
        (nil)))

(insn 39 42 40 0 (parallel [
            (set (reg:SI 2 cx)
                (const_int 0 [0x0]))
            (clobber (reg:CC 17 flags))
        ]) -1 (nil)
    (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))
[snip]

Can the result of a peephole2 be used in matching another peephole2? 
If yes in that case another peephole2 can be written to match 
(set REGXXX 0) ;; this is generated by a previous peephole2
(set (mem.... ) (const_int 0))

and change that to
(set (mem ....) REGXXXX)







-- 


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

[Bug target/23102] extra XORs generated on i686

[rguenth at gcc dot gnu dot org]

------- Additional Comments From rguenth at gcc dot gnu dot org  2005-08-04 11:54 -------
Try killing the peephole2 for this.  Somebody elses cost-metric should force
the constant into a register for the move ...

-- 


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

[Bug target/23102] extra XORs generated on i686

[dann at godzilla dot ics dot uci dot edu]

------- Additional Comments From dann at godzilla dot ics dot uci dot edu  2005-08-04 17:06 -------
(In reply to comment #3)
> Try killing the peephole2 for this.  Somebody elses cost-metric should force
> the constant into a register for the move ...

Killing the peephole2 would just produce the code produced for i386 as shown in
the original report.
What we want is something like 
xorl %ecx, %ecx
movl    %ecx, 3144(%eax)
movl    %ecx, 3124(%eax)
... etc etc


-- 


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

[Bug target/23102] extra XORs generated on i686

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-08-12 05:25 -------
The reason why GCSE after reload does not catch this is because the splitting happens after "gcse after 
reload" happens.

This seems like it should not be a peephole2 at all but a split_and_insn pattern.

-- 


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