Re: i386 string compare - code quality regression from 2.95

["Zack Weinberg" <zackw at Stanford dot EDU>]

On Mon, May 14, 2001 at 11:18:51PM -0700, Richard Henderson wrote:
> On Mon, May 14, 2001 at 11:11:23PM -0700, Zack Weinberg wrote:
> > So this would be the correct pattern?
> 
> Yes.

Thanks.

...
> > Could I get away with leaving out the modes altogether?
> 
> Yeah.  These are supposed to be valid instructions being input.
> Unlike with define_insn, define_split and define_peephole2 can
> afford to (also) match things that shouldn't exist.

Okay.

I looked a bit more closely at the (rare) cases I found where the
peephole doesn't trigger.  The cc modes aren't the problem.  One case
is an oversight - the string comparison pattern can look like

(parallel[ 
            (set (reg:CC 17 flags)
                (if_then_else:CC (ne (reg:SI 2 ecx [169])
                        (const_int 0 [0x0]))
                    (compare:CC (mem:BLK (reg/f:SI 4 esi [167]) 0)
                        (mem:BLK (reg/f:SI 5 edi [168]) 0))
                    (const_int 0 [0x0])))
	     ; clobbers + uses ...
	])

which is of course not matched.  This is the pattern for memcmp with
non-constant third argument.  It may or may not be worth writing
another peephole to match it, it doesn't happen that often.

The other case looks like this:

(insn:QI (parallel[ 
            (set (reg:CC 17 flags)
                (compare:CC (mem:BLK (reg/f:SI 4 esi [123]) 0)
                    (mem:BLK (reg/f:SI 5 edi [124]) 0)))
            (use (reg:SI 2 ecx [125]))
            (use (const_int 1 [0x1]))
            (use (reg:SI 19 dirflag))
            (clobber (reg/f:SI 4 esi [123]))
            (clobber (reg/f:SI 5 edi [124]))
            (clobber (reg:SI 2 ecx [125]))
        ]) {cmpstrqi_nz_1})

(insn:HI (set (reg:QI 1 dl [126])
        (gtu:QI (reg:CC 17 flags)
            (const_int 0 [0x0]))) {*setcc_1})

(insn (set (reg:QI 0 al [127])
        (ltu:QI (reg:CC 17 flags)
            (const_int 0 [0x0]))) {*setcc_1})

(note NOTE_INSN_DELETED)
(note NOTE_INSN_DELETED)
(note NOTE_INSN_BLOCK_END)

(insn (set (reg/v:SI 3 ebx [61])
        (const_int 4 [0x4])) (*movsi_1})

(insn:QI (set (reg:CCZ 17 flags)
        (compare:CCZ (reg:QI 1 dl [126])
            (reg:QI 0 al [127]))) {*cmpqi_1})

This is highly stereotyped - down to the modes on the insns.   There's
always two NOTE_INSN_DELETEDs, one NOTE_INSN_BLOCK_END, and one set of
ebx in between the setccs and the cmpqi.  The value ebx is set to varies.

Now, I'd thought that peephole2 was clever enough to ignore
intervening insns, and if it isn't, it's probably not worth writing
yet another peephole to match this case - diminishing returns.  But I
would like to know where the intervening instructions came from, when
we don't do pre-register allocation scheduling.  reload, maybe?  And
what's a NOTE_INSN_BLOCK_END doing in the middle of a block???

I'm looking at the .peephole2 dump from this:

$ ./xgcc -B./ -c -O2 -DIN_GCC -g -W -Wall -Wwrite-strings
-Wstrict-prototypes -Wmissing-prototypes -Wtraditional -pedantic
-Wno-long-long -DHAVE_CONFIG_H -I. -I. -I../../../gcc_vanilla/gcc
-I../../../gcc_vanilla/gcc/. -I../../../gcc_vanilla/gcc/config
-I../../../gcc_vanilla/gcc/../include
../../../gcc_vanilla/gcc/dwarf2out.c -o dwarf2out.o -dwz
-fdump-unnumbered

where ./cc1 has been compiled with the new peephole...

-- 
zw  But then one day I came up with a radical new paradigm for my business...
    I decided that from now on I would only sell boring stuff that people
    actually need.
    	-- Garry Trudeau, _Doonesbury_