From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28961 invoked by alias); 10 Apr 2005 02:43:55 -0000 Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-bugs-owner@gcc.gnu.org Received: (qmail 28843 invoked by alias); 10 Apr 2005 02:43:43 -0000 Date: Sun, 10 Apr 2005 02:43:00 -0000 Message-ID: <20050410024343.28842.qmail@sourceware.org> From: "aoliva at redhat dot com" To: gcc-bugs@gcc.gnu.org In-Reply-To: <20050221214433.20126.jkohen@users.sourceforge.net> References: <20050221214433.20126.jkohen@users.sourceforge.net> Reply-To: gcc-bugzilla@gcc.gnu.org Subject: [Bug target/20126] [3.3/3.4/4.0/4.1 Regression] Inlined memcmp makes one argument null on entry X-Bugzilla-Reason: CC X-SW-Source: 2005-04/txt/msg01187.txt.bz2 List-Id: ------- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-10 02:43 ------- Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail On Apr 8, 2005, Roger Sayle wrote: > ++ /* If there isn't a volatile MEM, there's nothing we can do. */ > ++ || !for_each_rtx (&object, volatile_mem_p, 0) This actually caused crashes. We don't want to scan the entire insn (it might contain NULLs), only the insn pattern. Here's the patch that bootstrapped and passed regression testing on amd64-linux-gnu. Ok? Index: gcc/ChangeLog from Alexandre Oliva PR target/20126 * loop.c (loop_givs_rescan): If replacement of DEST_ADDR failed, set the original address pseudo to the correct value before the original insn, if possible, and leave the insn alone, otherwise create a new pseudo, set it and replace it in the insn. * recog.c (validate_change_maybe_volatile): New. * recog.h (validate_change_maybe_volatile): Declare. Index: gcc/loop.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/loop.c,v retrieving revision 1.525 diff -u -p -r1.525 loop.c --- gcc/loop.c 2 Apr 2005 16:53:38 -0000 1.525 +++ gcc/loop.c 10 Apr 2005 00:13:56 -0000 @@ -5476,9 +5476,31 @@ loop_givs_rescan (struct loop *loop, str mark_reg_pointer (v->new_reg, 0); if (v->giv_type == DEST_ADDR) - /* Store reduced reg as the address in the memref where we found - this giv. */ - validate_change (v->insn, v->location, v->new_reg, 0); + { + /* Store reduced reg as the address in the memref where we found + this giv. */ + if (validate_change_maybe_volatile (v->insn, v->location, + v->new_reg)) + /* Yay, it worked! */; + /* Not replaceable; emit an insn to set the original + giv reg from the reduced giv. */ + else if (REG_P (*v->location)) + loop_insn_emit_before (loop, 0, v->insn, + gen_move_insn (*v->location, + v->new_reg)); + else + { + /* If it wasn't a reg, create a pseudo and use that. */ + rtx reg, seq; + start_sequence (); + reg = force_reg (v->mode, *v->location); + seq = get_insns (); + end_sequence (); + loop_insn_emit_before (loop, 0, v->insn, seq); + if (!validate_change_maybe_volatile (v->insn, v->location, reg)) + gcc_unreachable (); + } + } else if (v->replaceable) { reg_map[REGNO (v->dest_reg)] = v->new_reg; Index: gcc/recog.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/recog.c,v retrieving revision 1.221 diff -u -p -r1.221 recog.c --- gcc/recog.c 7 Mar 2005 13:52:08 -0000 1.221 +++ gcc/recog.c 10 Apr 2005 00:13:57 -0000 @@ -235,6 +235,46 @@ validate_change (rtx object, rtx *loc, r return apply_change_group (); } + +/* Function to be passed to for_each_rtx to test whether a piece of + RTL contains any mem/v. */ +static int +volatile_mem_p (rtx *x, void *data ATTRIBUTE_UNUSED) +{ + return (MEM_P (*x) && MEM_VOLATILE_P (*x)); +} + +/* Same as validate_change, but doesn't support groups, and it accepts + volatile mems if they're already present in the original insn. */ + +int +validate_change_maybe_volatile (rtx object, rtx *loc, rtx new) +{ + int result; + + if (validate_change (object, loc, new, 0)) + return 1; + + if (volatile_ok + /* If there isn't a volatile MEM, there's nothing we can do. */ + || !for_each_rtx (&PATTERN (object), volatile_mem_p, 0) + /* Make sure we're not adding or removing volatile MEMs. */ + || for_each_rtx (loc, volatile_mem_p, 0) + || for_each_rtx (&new, volatile_mem_p, 0) + || !insn_invalid_p (object)) + return 0; + + volatile_ok = 1; + + gcc_assert (!insn_invalid_p (object)); + + result = validate_change (object, loc, new, 0); + + volatile_ok = 0; + + return result; +} + /* This subroutine of apply_change_group verifies whether the changes to INSN were valid; i.e. whether INSN can still be recognized. */ Index: gcc/recog.h =================================================================== RCS file: /cvs/gcc/gcc/gcc/recog.h,v retrieving revision 1.55 diff -u -p -r1.55 recog.h --- gcc/recog.h 7 Mar 2005 13:52:09 -0000 1.55 +++ gcc/recog.h 10 Apr 2005 00:13:57 -0000 @@ -74,6 +74,7 @@ extern void init_recog_no_volatile (void extern int check_asm_operands (rtx); extern int asm_operand_ok (rtx, const char *); extern int validate_change (rtx, rtx *, rtx, int); +extern int validate_change_maybe_volatile (rtx, rtx *, rtx); extern int insn_invalid_p (rtx); extern void confirm_change_group (void); extern int apply_change_group (void); Index: gcc/testsuite/ChangeLog from Alexandre Oliva * gcc.dg/pr20126.c: New. Index: gcc/testsuite/gcc.dg/pr20126.c =================================================================== RCS file: gcc/testsuite/gcc.dg/pr20126.c diff -N gcc/testsuite/gcc.dg/pr20126.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ gcc/testsuite/gcc.dg/pr20126.c 10 Apr 2005 00:14:15 -0000 @@ -0,0 +1,50 @@ +/* dg-do run */ +/* dg-options "-O2" */ + +/* PR target/20126 was not really target-specific, but rather a loop's + failure to take into account the possibility that a DEST_ADDR giv + replacement might fail, such as when you attempt to replace a REG + with a PLUS in one of the register_operands of cmpstrqi_rex_1. */ + +extern void abort (void); + +typedef struct { int a; char b[3]; } S; +S c = { 2, "aa" }, d = { 2, "aa" }; + +void * +bar (const void *x, int y, int z) +{ + return (void *) 0; +} + +int +foo (S *x, S *y) +{ + const char *e, *f, *g; + int h; + + h = y->a; + f = y->b; + e = x->b; + + if (h == 1) + return bar (e, *f, x->a) != 0; + + g = e + x->a - h; + while (e <= g) + { + const char *t = e + 1; + if (__builtin_memcmp (e, f, h) == 0) + return 1; + e = t; + } + return 0; +} + +int +main (void) +{ + if (foo (&c, &d) != 1) + abort (); + return 0; +} -- Alexandre Oliva http://www.ic.unicamp.br/~oliva/ Red Hat Compiler Engineer aoliva@{redhat.com, gcc.gnu.org} Free Software Evangelist oliva@{lsd.ic.unicamp.br, gnu.org} -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126