[PATCH] cse (canon_reg): Don't modify insns without using validate_change

[Andreas Krebbel]

Hi,

> Maybe a little bit of additional testing would be useful - what I 
> usually do as a sanity check for such changes is to just compile a few 
> preprocessed files with patched and unpatched compilers and see if the 
> output differs anywhere.  If you could do that on an i686-linux target, 
> and show no differences, then that would be an argument that the patch 
> is likely safe for the branch.

Unfortunately I indeed found differences which are related to the patch.
I've not yet looked into the differences on i686 but I can describe what
happened on s390.

On s390 all the differences I've looked into where caused by additional
clobbers appearing the first time during cse. The issue depends heavily on
the machine description file so it is probably worth to have a look into 
the i686 cases as well. 

The s390 md file contains two insn patterns which (in case of register 
operands) match each other except that the first comes with an additional 
clobber of a scratch value. The initial call to recog chooses the second pattern
(the one without the clobber). It skips the first because it is not allowed to 
add clobbers. cse then wants to replace one pseudo in that insn with another 
and with my patch calls validate_change to do so. validate_change calls recog again
and allows recog to add clobbers for matching. recog finds the first insn pattern 
and decides to add a clobber to make it match.

Although the additional clobber does not itself lead to code differences it
in my expamples circumvents optimizations during combine. 

I'm not sure why combine stumbles on additional clobbers. As well as recog is able
to add clobbers to match a particular insn combine should be able to drop them when
necessary - right?!

Bye,

-Andreas-


Here is the example I've described:

patched:

(insn 307 306 308 10 /build/gcc-4.1/gcc/expmed.c:227 (parallel [
            (set (reg/v:SI 55 [ wider_mode ])
                (and:SI (subreg:SI (reg:QI 68 [ D.14766 ]) 0)
                    (const_int 255 [0xff])))
            (clobber (reg:CC 33 %cc))
        ]) -1 (insn_list:REG_DEP_TRUE 306 (nil))
    (expr_list:REG_UNUSED (reg:CC 33 %cc)
        (nil)))

(note 308 307 310 10 ("/build/gcc-4.1/gcc/expmed.c") 228)

(insn 310 308 311 10 /build/gcc-4.1/gcc/expmed.c:228 (parallel [
            (set (reg:CCZ 33 %cc)
                (compare:CCZ (reg/v:SI 55 [ wider_mode ])
                    (const_int 0 [0x0])))
            (clobber (scratch:SI))
        ]) -1 (insn_list:REG_DEP_TRUE 307 (nil))
    (expr_list:REG_UNUSED (scratch:SI)
        (nil)))

307 can't be combined with 310 due to the (clobber (scratch:SI)) in 310


nopatch:

(insn 307 306 308 10 /build/gcc-4.1/gcc/expmed.c:227 (parallel [
            (set (reg/v:SI 55 [ wider_mode ])
                (and:SI (subreg:SI (reg:QI 68 [ D.14766 ]) 0)
                    (const_int 255 [0xff])))
            (clobber (reg:CC 33 %cc))
        ]) 265 {*andsi3_zarch} (insn_list:REG_DEP_TRUE 306 (nil))
    (expr_list:REG_UNUSED (reg:CC 33 %cc)
        (nil)))

(note 308 307 310 10 ("/build/gcc-4.1/gcc/expmed.c") 228)

(insn 310 308 311 10 /build/gcc-4.1/gcc/expmed.c:228 (set (reg:CCZ 33 %cc)
        (compare:CCZ (reg/v:SI 55 [ wider_mode ])
            (const_int 0 [0x0]))) 15 {*tstsi_cconly2} (insn_list:REG_DEP_TRUE 307 (nil))
    (nil))

Without the clobber combine is able to do its job:

(insn 310 308 311 10 /build/gcc-4.1/gcc/expmed.c:228 (parallel [
            (set (reg:CCZ 33 %cc)
                (compare:CCZ (and:SI (subreg:SI (reg:QI 68 [ D.14766 ]) 0)
                        (const_int 255 [0xff]))
                    (const_int 0 [0x0])))
            (set (reg/v:SI 55 [ wider_mode ])
                (and:SI (subreg:SI (reg:QI 68 [ D.14766 ]) 0)
                    (const_int 255 [0xff])))
        ]) 263 {*andsi3_cc} (insn_list:REG_DEP_TRUE 306 (nil))
    (nil))