Re: gcc i686-pc-linux-gnu miscompiles small file at -O3 (latest cvs source)

[Zack Weinberg <zack at bitmover dot com>]

Jim Meyering wrote:
> When using the very latest gcc sources from cvs,
> I found that part of bash is miscompiled at -O3, but not at -O2.
> Here are distilled inputs that demonstrate the same problem.

I've done some basic analysis of a slightly adjusted version of Jim's
test case, which is now gcc.c-torture/execute/991112-1.c.

The problem is that with -finline-functions, the instruction to zero-
extend the return value of rl_character_len gets lost.  It appears to
be an interaction between a semi-bug in RTL generation and a genuine
bug in combine.

Without -finline-functions, the key sequence (after the first jump
pass) goes like this:

(insn 45 43 46 (set (reg:CCNO 17 flags)
        (compare:CCNO (reg:SI 28)
            (const_int 0 [0x0]))) -1 (nil)
    (nil))

(insn 46 45 47 (set (reg:QI 30)
        (eq:QI (reg:CCNO 17 flags)
            (const_int 0 [0x0]))) -1 (nil)
    (nil))

(insn 47 46 48 (parallel[ 
            (set (reg:SI 29)
                (zero_extend:SI (reg:QI 30)))
            (clobber (reg:CC 17 flags))
        ] ) -1 (nil)
    (nil))

Which is just fine.  With -finline-functions, the only change at
this point is that insns 46 and 47 now read

(insn 46 45 47 (set (subreg:QI (reg:SI 29) 0)
        (eq:QI (reg:CCNO 17 flags)
            (const_int 0 [0x0]))) -1 (nil)
    (nil))

(insn 47 46 48 (parallel[ 
            (set (reg:SI 29)
                (zero_extend:SI (subreg:QI (reg:SI 29) 0)))
            (clobber (reg:CC 17 flags))
        ] ) -1 (nil)
    (nil))

Look closely: reg 30 has disappeared.  This is the semi-bug:
-finline-functions should have made no difference in initial pseudo
allocation.  (No actual inlining occurs in this file.)  However, there
is nothing wrong with recycling reg 29 like this, and we proceed
merrily along until we hit the combiner.

Before combine:

(insn 46 45 47 (set (subreg:QI (reg:SI 29) 0)
        (eq:QI (reg:CCNO 17 flags)
            (const_int 0 [0x0]))) 363 {*setcc_1} (insn_list 45 (nil))
    (expr_list:REG_DEAD (reg:CCNO 17 flags)
        (nil)))

(insn 47 46 48 (parallel[ 
            (set (reg:SI 29)
                (zero_extend:SI (subreg:QI (reg:SI 29) 0)))
            (clobber (reg:CC 17 flags))
        ] ) 96 {zero_extendqisi2} (insn_list 46 (nil))
    (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))

After:

(insn 46 45 47 (set (subreg:QI (reg:SI 29) 0)
        (eq:QI (reg:CCNO 17 flags)
            (const_int 0 [0x0]))) 363 {*setcc_1} (insn_list 45 (nil))
    (expr_list:REG_DEAD (reg:CCNO 17 flags)
        (nil)))

(note 47 46 48 "" NOTE_INSN_DELETED)

The combiner apparently thinks that

(set (reg:SI X) (zero_extend:SI (subreg:QI (reg:SI X))

is a no-op.  This is the real bug.

#    Storing in a non-paradoxical `subreg' has undefined results for
#    bits belonging to the same word as the `subreg'.  This laxity makes
#    it easier to generate efficient code for such instructions.  To
#    represent an instruction that preserves all the bits outside of
#    those in the `subreg', use `strict_low_part' around the `subreg'.

insn 46 implicitly clobbers the high 24 bits of reg 29, so the
combiner is wrong to delete insn 47.

Again, the test case is gcc.c-torture/execute/991112-1.c.  I did my
testing with the 19991107 snapshot.  The current CVS tree doesn't
bootstrap for me, but I doubt it's fixed since.

zw