* RFA: Wrong code due to libcall CSE
@ 2003-04-02 15:19 Ulrich Weigand
0 siblings, 0 replies; only message in thread
From: Ulrich Weigand @ 2003-04-02 15:19 UTC (permalink / raw)
To: gcc
Hello,
we're seeing a miscompile (wrong code generated) with gcc 3.3 and 3.2.3
on s390-ibm-linux of the following testcase.
This appears to be caused by a non-trivial interaction of libcalls,
addressofs, REG_EQUAL notes, and cse. I'm not sure how to best fix
the bug; I'd be grateful for any advice ...
extern int strcspn (__const char *__s, __const char *__reject)
__attribute__ ((__pure__));
void test(void)
{
char *val = 0;
const char delim[] = " \t=";
char buff[200];
fill_buff (buff);
if (strcspn (buff, delim) < 10)
{
val = buff + strcspn (buff, delim);
*val++ = '\0';
val += strspn (buff, delim);
}
use_val (val);
}
The resulting code is incorrect in that the call to strspn uses an
uninitialized pseudo as second argument (instead of delim). This
comes to be because cse has replaced the insn loading the address
of delim inside the strspn libcall by simply reusing a pseudo
holding the address of delim which was set up as part of the
strcspn libcall, and a subsequent optimization has then deleted
the whole strscpn libcall (as duplicate pure function call)
together with the insn setting up that pseudo.
Now, from reading cse/gcse code it would appear that temporary
pseudos initialized inside a libcall block are not supposed to
be used as equivalences for cse, for exactly the reason that
the libcall block can be deleted as a whole.
It happens anyway in this case due to an interaction of REG_EQUAL
notes and addressof elimination.
That strcspn libcall originally (00.rtl) looks like this:
(insn 29 31 30 (nil) (set (reg:SI 2 %r2)
(reg/f:SI 36 virtual-stack-vars)) -1 (nil)
(insn_list:REG_LIBCALL 33 (nil)))
(insn 30 29 32 (nil) (set (reg:SI 3 %r3)
(addressof:SI (reg/v/u:SI 42) 41 0x401afd00)) -1 (nil)
(nil))
(call_insn/u 32 30 33 (nil) (parallel [
(set (reg:SI 2 %r2)
(call (mem:QI (reg:SI 48) [0 S1 A8])
(const_int 0 [0x0])))
(clobber (reg:SI 14 %r14))
]) -1 (nil)
(expr_list:REG_EH_REGION (const_int -1 [0xffffffffffffffff])
(nil))
(expr_list (use (mem:BLK (scratch) [0 A8]))
(expr_list (use (reg:SI 3 %r3))
(expr_list (use (reg:SI 2 %r2))
(nil)))))
(insn 33 32 34 (nil) (set (reg:SI 49)
(reg:SI 2 %r2)) -1 (nil)
(insn_list:REG_RETVAL 29 (expr_list:REG_EQUAL (expr_list (use (mem:BLK (scratch) [0 A8]))
(expr_list (symbol_ref:SI ("strcspn"))
(expr_list (addressof:SI (reg/v/u:SI 42) 41 0x401afd00)
(expr_list (reg/f:SI 36 virtual-stack-vars)
(nil)))))
(nil))))
The first CSE pass then adds a (redundant) REG_EQUAL note (09.cse):
(insn 29 58 30 1 0x401b0cc0 (set (reg:SI 2 %r2)
(reg/f:SI 57)) 56 {*movsi} (nil)
(expr_list:REG_EQUAL (plus:SI (reg/f:SI 34 %fp)
(const_int 96 [0x60]))
(insn_list:REG_LIBCALL 33 (nil))))
(insn 30 29 32 1 0x401b0cc0 (set (reg:SI 3 %r3)
(addressof:SI (reg/v/u:SI 42) 41 0x401afd00)) 56 {*movsi} (nil)
(expr_list:REG_EQUAL (addressof:SI (reg/v/u:SI 42) 41 0x401afd00)
(nil)))
(call_insn/u 32 30 33 1 0x401b0cc0 (parallel [
(set (reg:SI 2 %r2)
(call (mem:QI (reg/f:SI 45) [0 S1 A8])
(const_int 0 [0x0])))
(clobber (reg:SI 14 %r14))
]) 276 {basr_r_31} (nil)
(expr_list:REG_EH_REGION (const_int -1 [0xffffffffffffffff])
(nil))
(expr_list (use (mem:BLK (scratch) [0 A8]))
(expr_list (use (reg:SI 3 %r3))
(expr_list (use (reg:SI 2 %r2))
(nil)))))
(insn 33 32 35 1 0x401b0cc0 (set (reg:SI 49)
(reg:SI 46)) 56 {*movsi} (nil)
(insn_list:REG_RETVAL 29 (expr_list:REG_EQUAL (expr_list (use (mem:BLK (scratch) [0 A8]))
(expr_list (symbol_ref:SI ("strcspn"))
(expr_list (addressof:SI (reg/v/u:SI 42) 41 0x401afd00)
(expr_list (plus:SI (reg/f:SI 34 %fp)
(const_int 96 [0x60]))
(nil)))))
(nil))))
Addressof elimination now updates that REG_EQUAL note (10.addressof):
(insn 29 58 67 1 0x401b0cc0 (set (reg:SI 2 %r2)
(reg/f:SI 57)) 56 {*movsi} (nil)
(expr_list:REG_EQUAL (plus:SI (reg/f:SI 34 %fp)
(const_int 96 [0x60]))
(insn_list:REG_LIBCALL 33 (nil))))
(insn 67 29 30 1 (nil) (set (reg/f:SI 63)
(plus:SI (reg/f:SI 34 %fp)
(const_int 296 [0x128]))) 134 {*la_31} (nil)
(nil))
(insn 30 67 32 1 0x401b0cc0 (set (reg:SI 3 %r3)
(reg/f:SI 63)) 56 {*movsi} (nil)
(expr_list:REG_EQUAL (plus:SI (reg/f:SI 34 %fp)
(const_int 296 [0x128]))
(nil)))
(call_insn/u 32 30 33 1 0x401b0cc0 (parallel [
(set (reg:SI 2 %r2)
(call (mem:QI (reg/f:SI 45) [0 S1 A8])
(const_int 0 [0x0])))
(clobber (reg:SI 14 %r14))
]) 276 {basr_r_31} (nil)
(expr_list:REG_EH_REGION (const_int -1 [0xffffffffffffffff])
(nil))
(expr_list (use (mem:BLK (scratch) [0 A8]))
(expr_list (use (reg:SI 3 %r3))
(expr_list (use (reg:SI 2 %r2))
(nil)))))
(insn 33 32 35 1 0x401b0cc0 (set (reg:SI 49)
(reg:SI 46)) 56 {*movsi} (nil)
(insn_list:REG_RETVAL 29 (expr_list:REG_EQUAL (expr_list (use (mem:BLK (scratch) [0 A8]))
(expr_list (symbol_ref:SI ("strcspn"))
(expr_list (plus:SI (reg/f:SI 34 %fp)
(const_int 296 [0x128]))
(expr_list (plus:SI (reg/f:SI 34 %fp)
(const_int 96 [0x60]))
(nil)))))
(nil))))
After the GCSE pass, register 63 is used in the strspn call below.
This replacement does not happen within GCSE itself, but in the
re-run of CSE after GCSE. Now, CSE also has code that tries to
prevent building up equivalences to pseudos set inside libcall blocks,
and therefore this insn:
(insn 67 29 30 1 (nil) (set (reg/f:SI 63)
(plus:SI (reg/f:SI 34 %fp)
(const_int 296 [0x128]))) 134 {*la_31} (nil)
(nil))
does *not* establish the equivalence. However, the following:
(insn 30 67 32 1 0x401b0cc0 (set (reg:SI 3 %r3)
(reg/f:SI 63)) 56 {*movsi} (nil)
(expr_list:REG_EQUAL (plus:SI (reg/f:SI 34 %fp)
(const_int 296 [0x128]))
(nil)))
*does* set up this equivalence, because the SET_SRC and the REG_EQUAL
note are always entered into the CSE table even for instructions
inside a libcall ...
Interestingly enough, CVS head does not show this bug in this test case,
because the initial CSE run does not add the REG_EQUAL note. This is
due to the following patch by Roger Sayle:
http://gcc.gnu.org/ml/gcc-patches/2003-01/msg02173.html
which prevents creation of redundant REG_EQUAL notes.
However, this patch was not in fact intended as a bug fix but just
a compile-time optimization; furthermore, even with the patch I think
it still would be possible to get (non-redundant) REG_EQUAL notes
created (in other testcases), exposing that bug ...
Do you think Roger's patch is a proper fix, and should be backported
to 3.3 (and maybe 3.2.3)? Or can this problem be fixed inside CSE
in some other manner?
Thanks,
Ulrich
--
Dr. Ulrich Weigand
weigand@informatik.uni-erlangen.de
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2003-04-02 14:37 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-04-02 15:19 RFA: Wrong code due to libcall CSE Ulrich Weigand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).