This is a reduced testcase from a failure I am seeing on libgcj testsuite. java.lang.Class.isPrimitive() is being compiled incorrectly. This is a regression from 3.4.3. Configured as: ../clean/configure --target=mipsel-linux --with-sysroot=/usr/local/mipsel-linux-test --prefix=/usr/local/mipsel-linux-test --with-arch=mips32 --with-float=soft --with-system-zlib --disable-java-awt --without-x --disable-libmudflap --disable-libgomp --disable-jvmpi --disable-tls --enable-languages=c,c++,java on i686-pc-linux-gnu Here is the test case in C isprim.c: -------------8<------------ struct F; struct C { struct F *f13; }; int isM1(struct C *t) { return t->f13 == (struct F*)-1; } -------------8<------------ $ mipsel-linux-gcc -O1 -S isprim.c -o isprim.s Generates this correct code: lw $2,0($4) addiu $2,$2,1 j $31 sltu $2,$2,1 $ mipsel-linux-gcc -fnon-call-exceptions -O1 -S isprim.c -o isprim.s Generates this incorrect code that unconditionally returns 0: lw $2,0($4) .set noreorder .set nomacro j $31 move $2,$0 .set macro .set reorder
Created attachment 12465 [details] Dumps from -da with and without -fnon-call-exceptions The dump files in da-dumps.tgz named isprimb.c.* are from: $ mipsel-linux-gcc -fnon-call-exceptions -da -O1 -S isprimb.c -o isprimb.s Where isprimb.c is the exact same testcase as isprim.c The 'b' suffix indicating bad. The files named isprim.c.* are from: $ mipsel-linux-gcc -da -O1 -S isprim.c -o isprim.s
Expand does: ;; return t->f13 == 4294967295B (insn 10 9 11 (set (reg/f:SI 196) (mem/s/f/j:SI (reg/v/f:SI 194 [ t ]) [0 <variable>.f13+0 S4 A32])) -1 (nil) (nil)) (insn 11 10 12 (set (reg:SI 198) (plus:SI (reg/f:SI 196) (const_int 1 [0x1]))) -1 (nil) (nil)) (insn 12 11 13 (set (reg:SI 197) (eq:SI (reg:SI 198) (const_int 0 [0x0]))) -1 (nil) (nil)) (insn 13 12 14 (set (reg:SI 193 [ <result> ]) (reg:SI 197)) -1 (nil) (nil)) (jump_insn 14 13 15 (set (pc) (label_ref 0)) -1 (nil) (nil)) And then CSE1 changes that into: (insn 6 8 7 2 (set (reg/v/f:SI 194 [ t ]) (reg:SI 4 $4 [ t ])) 213 {*movsi_internal} (nil) (expr_list:REG_EQUIV (mem/f/c/i:SI (reg/f:SI 77 $arg) [0 t+0 S4 A32]) (nil))) (note 7 6 10 2 NOTE_INSN_FUNCTION_BEG) (insn 10 7 16 2 (set (reg/f:SI 196 [ <variable>.f13 ]) (mem/s/f/j:SI (reg/v/f:SI 194 [ t ]) [0 <variable>.f13+0 S4 A32])) 213 {*movsi_internal} (nil) (nil)) (note 16 10 19 2 NOTE_INSN_FUNCTION_END) (insn 19 16 20 2 (asm_input ("")) -1 (nil) (nil)) (insn 20 19 26 2 (set (reg/i:SI 2 $2 [ <result> ]) (const_int 0 [0x0])) 213 {*movsi_internal} (nil) (nil))
The difference between with and without -fnon-call-exceptions is: insn 10 9 11 3 (set (reg/f:SI 196) (mem/s/f/j:SI (reg/v/f:SI 194 [ t ]) [0 <variable>.f13+0 S4 A32])) -1 (nil) (nil)) vs: (insn 10 9 11 3 (set (reg:SI 196) (mem/s/f/j:SI (reg/v/f:SI 194 [ t ]) [0 <variable>.f13+0 S4 A32])) -1 (nil) (nil)) Note in the case where it is set, we record 196 as a pointer so we know that pointers don't wrap so we decide that pointer+1 can never be 0 in CSE.
Caused by commit r117143 Thanks to Andrew Pinski for helping me find it.
The wrapping check was added by: http://gcc.gnu.org/ml/gcc-patches/2002-10/msg00360.html I think this check is wrong for exactly the reason seen here; without type information, and without context, there's no guarantee that the PLUS itself is a pointer, or indeed that the constant was originally positive. We could only do something like this if we know the address is being dereferenced -- but in that case, we generally care about whether the value might trap, not whether it's zero (and rtlanal.c rightly says that (plus reg (const_int ...)) can trap). For example, something like: (uintptr_t) foo == 0x90000000 could legitimately be implemented as: (eq (plus foo (const_int 0x70000000)) (const_int 0)) And AIUI, gcc allows things like: (uintptr_t) foo - 0x90000000 and allows the result to be zero (assuming the result is not dereferenced). All uses of nonzero_address_p appear to using it for general rtx simplification, i.e. in cases where the value is not known to be a pointer. So I think this check should just be removed. Hopefully the check is less important now than it was when it was originally committed. This sort of optimisation can be done at the tree level instead. Richard
Created attachment 12474 [details] Potential fix. As per Richard's analysis, I am testing this patch. It seems to fix the testcase. I will bootstrap and test i686-pc-linux-gnu as well as mipsel-linux-gnu. Richard, does this look right?
Subject: Re: [4.2 Regression] Bad code on MIPS with -fnon-call-exceptions "daney at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes: > Richard, does this look right? Looks good to me.
The patch is here: http://gcc.gnu.org/ml/gcc-patches/2006-10/msg01149.html
Subject: Bug 29519 Author: daney Date: Wed Oct 25 05:49:43 2006 New Revision: 118023 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=118023 Log: PR middle-end/29519 * rtlanal.c (nonzero_address_p): Remove check for values wrapping. Modified: trunk/gcc/ChangeLog trunk/gcc/rtlanal.c
Subject: Bug 29519 Author: daney Date: Wed Oct 25 22:29:10 2006 New Revision: 118044 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=118044 Log: PR middle-end/29519 * rtlanal.c (nonzero_address_p): Remove check for values wrapping. Modified: branches/gcc-4_2-branch/gcc/ChangeLog branches/gcc-4_2-branch/gcc/rtlanal.c
With the (now committed) patches, my testing shows the problem as fixed.