From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14788 invoked by alias); 3 Oct 2007 15:01:42 -0000 Received: (qmail 14759 invoked by uid 22791); 3 Oct 2007 15:01:29 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 03 Oct 2007 15:01:25 +0000 Received: (qmail 10938 invoked from network); 3 Oct 2007 15:01:23 -0000 Received: from unknown (HELO localhost) (froydnj@127.0.0.2) by mail.codesourcery.com with ESMTPA; 3 Oct 2007 15:01:23 -0000 Date: Wed, 03 Oct 2007 15:01:00 -0000 From: Nathan Froyd To: gcc-patches@gcc.gnu.org Subject: [PATCH,i386] fix PR 11001 Message-ID: <20071003150123.GA23624@codesourcery.com> Mail-Followup-To: gcc-patches@gcc.gnu.org MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="x+6KMIRAuhnl3hBn" Content-Disposition: inline User-Agent: Mutt/1.5.13 (2006-08-11) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2007-10/txt/msg00181.txt.bz2 --x+6KMIRAuhnl3hBn Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 2075 The attached patch fixes PR11001, a long-standing bad interaction between register globals and strlen/memset/memcpy on the x86. The primary issue is that the expanders for those functions would generate (in some cases) x86 instructions that required particular registers without checking whether those registers were available for use. The fix is to ensure that the registers are available before generating the instructions. Note that the code is not optimal: in the memset case, for instance, if we choose an inlining strategy requiring 'rep stosl' and then discover that the necessary registers are not available, we generate a full call to 'memset' rather than generating an inline copy loop. I don't see this as a serious defect; if you are using register globals on the x86, you deserve a performance penalty. The testcase in the PR no longer ICEs on mainline--it does produce incorrect code, but that is mostly due to the semantics of parameter-scope register variables differing from global reigster variables and is therefore a bug in the code. It is not included. The nine testcases included in the patch *do* ICE on current mainline, however, and no longer ICE with the patch. Tested on x86_64-unknown-linux-gnu with no regressions. OK to commit? -Nathan 2007-10-03 Nathan Froyd PR 11001 gcc/ * config/i386/i386.md (strmov): Check for esi and edi usage. * config/i386/i386.c (ix86_expand_movmem): Check for ecx, esi, and edi usage. (ix86_expand_setmem): Check for eax, ecx, and edi usage. (ix86_expand_strlen): Likewise. gcc/testsuite/ * gcc.target/i386/pr11001-strlen-1.c: New testcase. * gcc.target/i386/pr11001-strlen-2.c: New testcase. * gcc.target/i386/pr11001-strlen-3.c: New testcase. * gcc.target/i386/pr11001-memset-1.c: New testcase. * gcc.target/i386/pr11001-memset-2.c: New testcase. * gcc.target/i386/pr11001-memset-3.c: New testcase. * gcc.target/i386/pr11001-memcpy-1.c: New testcase. * gcc.target/i386/pr11001-memcpy-2.c: New testcase. * gcc.target/i386/pr11001-memcpy-3.c: New testcase. --x+6KMIRAuhnl3hBn Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="pr11001.patch" Content-length: 9006 Index: gcc/config/i386/i386.md =================================================================== --- gcc/config/i386/i386.md (revision 128981) +++ gcc/config/i386/i386.md (working copy) @@ -18699,7 +18699,9 @@ (define_expand "strmov" operands[5] = gen_rtx_PLUS (Pmode, operands[0], adjust); operands[6] = gen_rtx_PLUS (Pmode, operands[2], adjust); - if (TARGET_SINGLE_STRINGOP || optimize_size) + /* Can't use this if the user has appropriated esi or edi. */ + if ((TARGET_SINGLE_STRINGOP || optimize_size) + && !(global_regs[4] || global_regs[5])) { emit_insn (gen_strmov_singleop (operands[0], operands[1], operands[2], operands[3], Index: gcc/config/i386/i386.c =================================================================== --- gcc/config/i386/i386.c (revision 128981) +++ gcc/config/i386/i386.c (working copy) @@ -15286,6 +15286,13 @@ ix86_expand_movmem (rtx dst, rtx src, rt break; } + /* Can't use this if the user has appropriated ecx, esi, or edi. */ + if ((alg == rep_prefix_1_byte + || alg == rep_prefix_4_byte + || alg == rep_prefix_8_byte) + && (global_regs[2] || global_regs[4] || global_regs[5])) + return 0; + epilogue_size_needed = size_needed; /* Step 1: Prologue guard. */ @@ -15594,6 +15601,14 @@ ix86_expand_setmem (rtx dst, rtx count_e size_needed = 1; break; } + + /* Can't use this if the user has appropriated eax, ecx, or edi. */ + if ((alg == rep_prefix_1_byte + || alg == rep_prefix_4_byte + || alg == rep_prefix_8_byte) + && (global_regs[0] || global_regs[2] || global_regs[5])) + return 0; + epilogue_size_needed = size_needed; /* Step 1: Prologue guard. */ @@ -15985,6 +16000,11 @@ ix86_expand_strlen (rtx out, rtx src, rt else { rtx unspec; + + /* Can't use this if the user has appropriated eax, ecx, or edi. */ + if (global_regs[0] || global_regs[2] || global_regs[5]) + return false; + scratch2 = gen_reg_rtx (Pmode); scratch3 = gen_reg_rtx (Pmode); scratch4 = force_reg (Pmode, constm1_rtx); Index: gcc/testsuite/gcc.target/i386/pr11001-strlen-2.c =================================================================== --- gcc/testsuite/gcc.target/i386/pr11001-strlen-2.c (revision 0) +++ gcc/testsuite/gcc.target/i386/pr11001-strlen-2.c (revision 0) @@ -0,0 +1,16 @@ +/* Ensure that we don't use 'repnz scasb' in the presence of register globals. */ +/* { dg-do compile } */ +/* { dg-options "-O1 -m32" } */ + +extern __SIZE_TYPE__ strlen (const char *); +extern void *malloc (__SIZE_TYPE__); + +register int regvar asm("%eax"); /* { dg-warning "call-clobbered register" } */ + +char * +do_copy (char *str) +{ + return malloc (strlen (str) + 1); +} + +/* { dg-final { scan-assembler-not "repnz scasb" } } */ Index: gcc/testsuite/gcc.target/i386/pr11001-memset-2.c =================================================================== --- gcc/testsuite/gcc.target/i386/pr11001-memset-2.c (revision 0) +++ gcc/testsuite/gcc.target/i386/pr11001-memset-2.c (revision 0) @@ -0,0 +1,23 @@ +/* Ensure that we don't use 'rep stoX' in the presence of register globals. */ +/* { dg-do compile } */ +/* { dg-options "-Os -m32" } */ + +extern void *memset (void *, int, __SIZE_TYPE__); + +register int regvar asm("%ecx"); /* { dg-warning "call-clobbered register" } */ + +int foo[10]; +int bar[10]; + +char baz[15]; +char quux[15]; + +void +do_copy () +{ + memset (foo, 0, sizeof foo); + memset (baz, 0, sizeof baz); +} + +/* { dg-final { scan-assembler-not "rep stosl" } } */ +/* { dg-final { scan-assembler-not "rep stosb" } } */ Index: gcc/testsuite/gcc.target/i386/pr11001-strlen-3.c =================================================================== --- gcc/testsuite/gcc.target/i386/pr11001-strlen-3.c (revision 0) +++ gcc/testsuite/gcc.target/i386/pr11001-strlen-3.c (revision 0) @@ -0,0 +1,16 @@ +/* Ensure that we don't use 'repnz scasb' in the presence of register globals. */ +/* { dg-do compile } */ +/* { dg-options "-O1 -m32" } */ + +extern __SIZE_TYPE__ strlen (const char *); +extern void *malloc (__SIZE_TYPE__); + +register int regvar asm("%ecx"); /* { dg-warning "call-clobbered register" } */ + +char * +do_copy (char *str) +{ + return malloc (strlen (str) + 1); +} + +/* { dg-final { scan-assembler-not "repnz scasb" } } */ Index: gcc/testsuite/gcc.target/i386/pr11001-memset-3.c =================================================================== --- gcc/testsuite/gcc.target/i386/pr11001-memset-3.c (revision 0) +++ gcc/testsuite/gcc.target/i386/pr11001-memset-3.c (revision 0) @@ -0,0 +1,23 @@ +/* Ensure that we don't use 'rep stoX' in the presence of register globals. */ +/* { dg-do compile } */ +/* { dg-options "-Os -m32" } */ + +extern void *memset (void *, int, __SIZE_TYPE__); + +register int regvar asm("%edi"); + +int foo[10]; +int bar[10]; + +char baz[15]; +char quux[15]; + +void +do_copy () +{ + memset (foo, 0, sizeof foo); + memset (baz, 0, sizeof baz); +} + +/* { dg-final { scan-assembler-not "rep stosl" } } */ +/* { dg-final { scan-assembler-not "rep stosb" } } */ Index: gcc/testsuite/gcc.target/i386/pr11001-memcpy-1.c =================================================================== --- gcc/testsuite/gcc.target/i386/pr11001-memcpy-1.c (revision 0) +++ gcc/testsuite/gcc.target/i386/pr11001-memcpy-1.c (revision 0) @@ -0,0 +1,23 @@ +/* Ensure that we don't use 'rep movX' in the presence of register globals. */ +/* { dg-do compile } */ +/* { dg-options "-Os -m32" } */ + +extern void *memcpy (void *, const void *, __SIZE_TYPE__); + +register int regvar asm("%esi"); + +int foo[10]; +int bar[10]; + +char baz[15]; +char quux[15]; + +void +do_copy () +{ + memcpy (foo, bar, sizeof foo); + memcpy (baz, quux, sizeof baz); +} + +/* { dg-final { scan-assembler-not "rep movsl" } } */ +/* { dg-final { scan-assembler-not "rep movsb" } } */ Index: gcc/testsuite/gcc.target/i386/pr11001-memcpy-2.c =================================================================== --- gcc/testsuite/gcc.target/i386/pr11001-memcpy-2.c (revision 0) +++ gcc/testsuite/gcc.target/i386/pr11001-memcpy-2.c (revision 0) @@ -0,0 +1,23 @@ +/* Ensure that we don't use 'rep movX' in the presence of register globals. */ +/* { dg-do compile } */ +/* { dg-options "-Os -m32" } */ + +extern void *memcpy (void *, const void *, __SIZE_TYPE__); + +register int regvar asm("%edi"); + +int foo[10]; +int bar[10]; + +char baz[15]; +char quux[15]; + +void +do_copy () +{ + memcpy (foo, bar, sizeof foo); + memcpy (baz, quux, sizeof baz); +} + +/* { dg-final { scan-assembler-not "rep movsl" } } */ +/* { dg-final { scan-assembler-not "rep movsb" } } */ Index: gcc/testsuite/gcc.target/i386/pr11001-memcpy-3.c =================================================================== --- gcc/testsuite/gcc.target/i386/pr11001-memcpy-3.c (revision 0) +++ gcc/testsuite/gcc.target/i386/pr11001-memcpy-3.c (revision 0) @@ -0,0 +1,23 @@ +/* Ensure that we don't use 'rep movX' in the presence of register globals. */ +/* { dg-do compile } */ +/* { dg-options "-Os -m32" } */ + +extern void *memcpy (void *, const void *, __SIZE_TYPE__); + +register int regvar asm("%ecx"); /* { dg-warning "call-clobbered register" } */ + +int foo[10]; +int bar[10]; + +char baz[15]; +char quux[15]; + +void +do_copy () +{ + memcpy (foo, bar, sizeof foo); + memcpy (baz, quux, sizeof baz); +} + +/* { dg-final { scan-assembler-not "rep movsl" } } */ +/* { dg-final { scan-assembler-not "rep movsb" } } */ Index: gcc/testsuite/gcc.target/i386/pr11001-strlen-1.c =================================================================== --- gcc/testsuite/gcc.target/i386/pr11001-strlen-1.c (revision 0) +++ gcc/testsuite/gcc.target/i386/pr11001-strlen-1.c (revision 0) @@ -0,0 +1,16 @@ +/* Ensure that we don't use 'repnz scasb' in the presence of register globals. */ +/* { dg-do compile } */ +/* { dg-options "-O1 -m32" } */ + +extern __SIZE_TYPE__ strlen (const char *); +extern void *malloc (__SIZE_TYPE__); + +register int regvar asm("%edi"); + +char * +do_copy (char *str) +{ + return malloc (strlen (str) + 1); +} + +/* { dg-final { scan-assembler-not "repnz scasb" } } */ Index: gcc/testsuite/gcc.target/i386/pr11001-memset-1.c =================================================================== --- gcc/testsuite/gcc.target/i386/pr11001-memset-1.c (revision 0) +++ gcc/testsuite/gcc.target/i386/pr11001-memset-1.c (revision 0) @@ -0,0 +1,23 @@ +/* Ensure that we don't use 'rep stoX' in the presence of register globals. */ +/* { dg-do compile } */ +/* { dg-options "-Os -m32" } */ + +extern void *memset (void *, int, __SIZE_TYPE__); + +register int regvar asm("%eax"); /* { dg-warning "call-clobbered register" } */ + +int foo[10]; +int bar[10]; + +char baz[15]; +char quux[15]; + +void +do_copy () +{ + memset (foo, 0, sizeof foo); + memset (baz, 0, sizeof baz); +} + +/* { dg-final { scan-assembler-not "rep stosl" } } */ +/* { dg-final { scan-assembler-not "rep stosb" } } */ --x+6KMIRAuhnl3hBn--