This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Problems with the way we calculate costs



Bernd Schmidt <bernds@redhat.co.uk> writes:

> Jeff Law has reviewed and approved this patch.  I'll check it in after another
> bootstrap on ix86.
> 
> Bernd
> 
> On Wed, 23 Aug 2000, Bernd Schmidt wrote:
> > 	* Makefile.in (cse.o): Depend on $(BASIC_BLOCK_H).
> > 	* cse.c: Include "basic-block.h".
> > 	(struct table_elt): New field REGCOST.
> > 	(CHEAP_REG): Delete macro.
> > 	(COST): Return 0 for REGs.
> > 	(approx_reg_cost_1, approx_reg_cost, preferrable): New functions.
> > 	(notreg_cost): Return 0 for appropriate SUBREGs.
> > 	(COSTS_N_INSNS): Return N * 2.
> > 	(rtx_cost): Return 0 for REGs, and use cost of nested rtx for cheap
> > 	SUBREGs.
> > 	(CHEAPER): Use new function preferrable.
> > 	(insert): Initialize REGCOST member.
> > 	(find_best_addr): Use approx_reg_cost for estimation of register
> > 	usage.
> > 	(cse_insn): Likewise.

Hi Bernd,
this patch causes the bootstrap to fail on powerpc-linux.

Part of the problem is that in the following code sequence:

(call_insn 968 967 970 (parallel[ 
            (set (reg:SI 3 r3)
                (call (mem:SI (symbol_ref:SI ("open_in_zip")) 0)
                    (const_int 0 [0x0])))
            (use (const_int 0 [0x0]))
            (clobber (scratch:SI))
        ] ) -1 (nil)
    (expr_list:REG_EH_REGION (const_int 0 [0x0])
        (nil))
    (expr_list (use (reg:SI 6 r6))
        (expr_list (use (reg:SI 5 r5))
            (expr_list (use (reg:SI 4 r4))
                (expr_list (use (reg:SI 3 r3))
                    (nil))))))

(insn 970 968 971 (set (reg/v:SI 369)
        (reg:SI 3 r3)) -1 (nil)
    (nil))

(note 971 970 972 ("/home/geoffk/cygnus/co/egcs-mainline/egcs/gcc/java/jcf-io.c") 362 -1347440721)

(insn 972 971 973 (set (reg:CC 375)
        (compare:CC (reg/v:SI 369)
            (const_int 0 [0x0]))) -1 (nil)
    (nil))

(jump_insn 973 972 974 (set (pc)
        (if_then_else (ne (reg:CC 375)
                (const_int 0 [0x0]))
            (label_ref 2317)
            (pc))) 459 {jump-8} (nil)
    (nil))

; CSE detects that at this point (reg:SI 3) must have value 0.
; with your patch CSE also decides that the best substitution for
; (const_int 0) is therefore (reg:SI 3), because of the change
; to the cost stuff.  CSE used to prefer (reg:SI 369).
<about 50 insns snipped>

(insn 1020 1018 1021 (set (reg:QI 395)
        (const_int 0 [0x0])) -1 (nil)
    (expr_list:REG_EQUAL (const_int 0 [0x0])
        (nil)))

; so whenever we see (reg:QI 395), we can use (reg:QI r3),
; because r3 contains (const_int 0) in a wider mode.  It used
; to prefer (subreg:QI (reg:SI 369)), but SUBREG isn't used
; for hard registers.

(insn 1021 1020 1023 (set (mem/s:QI (plus:SI (reg:SI 31 r31)
                (const_int 218 [0xda])) 0)
        (reg:QI 395)) 285 {*movsi_internal2+2} (nil)
    (nil))
...
(insn 1025 1023 1027 (set (reg:SI 397)
        (plus:SI (reg:SI 31 r31)
            (const_int 216 [0xd8]))) 36 {*addsi3_internal1} (nil)
    (nil))
...
(insn 1030 1027 1032 (set (reg:QI 399)
        (mem/s:QI (plus:SI (reg:SI 397)
                (const_int 2 [0x2])) 111)) -1 (nil)
    (nil))

; (reg:QI r3) was stored into this location earlier, so this gets
; turned into (set (reg:QI 399) (reg:QI 3)).  This is a register
; equivalence, but unfortunately CSE doesn't cope very well with
; registers that change modes (it's not allowed for pseudos)
; and so what CSE actually remembers is that
; register 399 is equivalent to (reg:SI 3).
; This is a bug.

...
(insn 1033 1032 1034 (set (mem/s:QI (plus:SI (reg:SI 396)
                (const_int 2 [0x2])) 0)
        (reg:QI 399)) -1 (nil)
    (nil))

; so CSE substitutes (reg:SI 3) in here, and the insn is unrecognizable
; because the modes don't match.


However, I don't like the way that CSE stretched the lifespan of r3
over so many insns in the first place, and would much rather this
didn't happen.  Explicit hard register uses like this are bad and
are likely to cause problems later in the compiler.

-- 
- Geoffrey Keating <geoffk@cygnus.com>

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]