From patchwork Wed Mar 21 21:45:58 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Harshit Chopra X-Patchwork-Id: 148073 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 87D69B6F62 for ; Thu, 22 Mar 2012 08:46:46 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1332971206; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Received:Received:To:Subject:Message-Id:Date: From:Mailing-List:Precedence:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:Sender:Delivered-To; bh=tDV3+P6 q0JI/unNyr7vgztUcMCQ=; b=Cnd6Shnir+L1SG3FZp1aVSaUC3VQ4eXQVPYdRCH tufk9qsSM/tAqtfIsM/JR17tdK7cbC4DpnOfrFBmmksd0gO/w17w6EW0WJ9a2z7R T0WhSo8La6BowPta/2Yn6dWLqXFyy/uEmwCyXKp0cH5xBULWEG4Fa7pXZykX290U PPXk= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:X-Google-DKIM-Signature:Received:Received:Received:Received:Received:To:Subject:Message-Id:Date:From:X-Gm-Message-State:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=kYzm+ee5SGm/Yu0BxCz5yavQDZ6ypmq2/KlEUJIwm/ggGli5FShMS9eUdzJAFP FQ75IOa0ZqrUvQqMrK10clQ45usOzJbpqSNw+bhhmwyaga4v3MHPFznM5rXEmvp5 SasogOOtL1ECvTn3XQW+90Z1KgeQkKoPUYnG7v6beTwqA=; Received: (qmail 2606 invoked by alias); 21 Mar 2012 21:46:33 -0000 Received: (qmail 2432 invoked by uid 22791); 21 Mar 2012 21:46:26 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_LOW, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-yw0-f73.google.com (HELO mail-yw0-f73.google.com) (209.85.213.73) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 21 Mar 2012 21:46:00 +0000 Received: by yhpp61 with SMTP id p61so249366yhp.2 for ; Wed, 21 Mar 2012 14:45:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=to:subject:message-id:date:from:x-gm-message-state; bh=nQ6L00VZuaVuAFQpyxYlsdiuWe92gLH1yiTQzsWf3ZI=; b=ZRNMaASHFZ/+edexMq0W2k3/xDL4fOK8dfsSu6BUQ8NQNjqfVhIY+FevOlmfKhVUbC i/rEw32oSdgUym2lRz32oUNc9rIuxGQ5yXF760K3U3PPVK6m+eB0dARfD2i6Fa/zb1xn VfjM0ktOj3SgvETdJYlBHz6ybJuuEW9vh5s5WRE5uXbngYQrHXQK++13SJO4UaFkiAg5 xqc4otNDDnqtvytPAZxFCTec2VrvzZoHRisuVGLnro1LOmoBGAvQk7WtL8r5IWe71sao oYISImkZaO4eFk+mLUORCeXhDNAwQwU/qiiJk+0ILs8Yd8tEddrf7vsXcEcIDx8eBnAA DlgA== Received: by 10.236.118.37 with SMTP id k25mr7102662yhh.3.1332366358925; Wed, 21 Mar 2012 14:45:58 -0700 (PDT) Received: by 10.236.118.37 with SMTP id k25mr7102645yhh.3.1332366358866; Wed, 21 Mar 2012 14:45:58 -0700 (PDT) Received: from wpzn3.hot.corp.google.com (216-239-44-65.google.com [216.239.44.65]) by gmr-mx.google.com with ESMTPS id y53si1794498yhe.4.2012.03.21.14.45.58 (version=TLSv1/SSLv3 cipher=AES128-SHA); Wed, 21 Mar 2012 14:45:58 -0700 (PDT) Received: from hchopra.mtv.corp.google.com (hchopra.mtv.corp.google.com [172.18.110.87]) by wpzn3.hot.corp.google.com (Postfix) with ESMTP id B0C2B10004D; Wed, 21 Mar 2012 14:45:58 -0700 (PDT) Received: by hchopra.mtv.corp.google.com (Postfix, from userid 48577) id 5543B4314A; Wed, 21 Mar 2012 14:45:58 -0700 (PDT) To: reply@codereview.appspotmail.com, davidxl@google.com, gcc-patches@gcc.gnu.org Subject: [google] Minor cleanup and test fixes for -mpatch-functions-for-instrumentation. (issue5877043) Message-Id: <20120321214558.5543B4314A@hchopra.mtv.corp.google.com> Date: Wed, 21 Mar 2012 14:45:58 -0700 (PDT) From: harshit@google.com (Harshit Chopra) X-Gm-Message-State: ALoCoQnxYlukUaUEtYsiZLhOjG+u3Zk9OBYYU76bgOi6hdx0bZDM1Nxo7fV8n/ATqDKt7ponr2zvkP8X3Wx9cudqAnOrMGVMiTLnw8Ku6KeKAWaBr0D48v65AneZ6FUBZED61ApZMNnsP7b2Q9BHuPWcVLddKDQbeEeadwUVZxz4P1yDMWGMHzo= Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 2012-03-21 Harshit Chopra Minor changes: i386.c: made check_should_patch_current_function C90 compatible. i386.md: Added '\t' to bytes generated by ix86_output_function_nops_prologue_epilogue for proper formatting of assembly. patch-functions-*.c: Fixed verification in tests. Added a test to verify nop-bytes generated for sibling calls and another test to verify a binary with nop-bytes runs properly. * gcc/config/i386/i386.c (check_should_patch_current_function): * gcc/config/i386/i386.md: * gcc/testsuite/gcc.target/i386/patch-functions-1.c (void foo): (int main): * gcc/testsuite/gcc.target/i386/patch-functions-2.c: * gcc/testsuite/gcc.target/i386/patch-functions-3.c: * gcc/testsuite/gcc.target/i386/patch-functions-4.c: * gcc/testsuite/gcc.target/i386/patch-functions-5.c: * gcc/testsuite/gcc.target/i386/patch-functions-6.c: * gcc/testsuite/gcc.target/i386/patch-functions-7.c: * gcc/testsuite/gcc.target/i386/patch-functions-8.c (int foo): (int bar): * gcc/testsuite/gcc.target/i386/patch-functions-sibling-call.c: Testing method: make check-gcc RUNTESTFLAGS="i386.exp=patch-functions* --target_board=\"unix{-m32,}\"" Patch to be applied to google/main. --- This patch is available for review at http://codereview.appspot.com/5877043 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 08bd5f0..be1f7a4 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -10981,6 +10981,7 @@ check_should_patch_current_function (void) const char* func_name = NULL; struct loops loops; int num_loops = 0; + int min_functions_instructions; /* Patch the function if it has at least a loop. */ if (!patch_functions_ignore_loops) @@ -11007,7 +11008,7 @@ check_should_patch_current_function (void) strcmp("main", func_name) == 0) return true; - int min_functions_instructions = + min_functions_instructions = PARAM_VALUE (PARAM_FUNCTION_PATCH_MIN_INSTRUCTIONS); if (min_functions_instructions > 0) { diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 08353ff..38a04ae 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -11688,7 +11688,7 @@ /* Emit 10 nop bytes after ret. */ if (ix86_output_function_nops_prologue_epilogue (asm_out_file, FUNCTION_PATCH_EPILOGUE_SECTION, - "ret", + "\tret", 10)) return ""; } @@ -11712,7 +11712,7 @@ /* Emit 9 nop bytes after rep;ret. */ if (ix86_output_function_nops_prologue_epilogue (asm_out_file, FUNCTION_PATCH_EPILOGUE_SECTION, - "rep\;ret", + "\trep\;ret", 9)) return ""; } diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-1.c b/gcc/testsuite/gcc.target/i386/patch-functions-1.c index 308e8c3..aa1f424 100644 --- a/gcc/testsuite/gcc.target/i386/patch-functions-1.c +++ b/gcc/testsuite/gcc.target/i386/patch-functions-1.c @@ -1,5 +1,5 @@ /* Verify -mpatch-functions-for-instrumentation works. */ -/* { dg-do run } */ +/* { dg-do compile } */ /* { dg-require-effective-target lp64 } */ /* { dg-options "-mpatch-functions-for-instrumentation" } */ @@ -8,13 +8,16 @@ /* Check nop-bytes at end. */ /* { dg-final { scan-assembler "ret(.*).byte\t0x90(.*).byte\t0x90" } } */ -void foo() { +__attribute__ ((noinline)) +void foo() +{ /* Dummy loop. */ int x = 0; while (++x); } -int main() { +int main() +{ foo(); return 0; } diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-2.c b/gcc/testsuite/gcc.target/i386/patch-functions-2.c index 6baad32..78de867 100644 --- a/gcc/testsuite/gcc.target/i386/patch-functions-2.c +++ b/gcc/testsuite/gcc.target/i386/patch-functions-2.c @@ -1,4 +1,4 @@ -/* { dg-do run } */ +/* { dg-do compile } */ /* { dg-require-effective-target lp64 } */ /* { dg-options "-mpatch-functions-for-instrumentation -mno-patch-functions-main-always" } */ @@ -8,11 +8,14 @@ /* { dg-final { scan-assembler-not ".byte\t0xeb,0x09(.*).byte\t0x90" } } */ /* { dg-final { scan-assembler-not "ret(.*).byte\t0x90(.*).byte\t0x90" } } */ -void foo() { +__attribute__ ((noinline)) +void foo() +{ int x = 0; } -int main() { +int main() +{ foo(); return 0; } diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-3.c b/gcc/testsuite/gcc.target/i386/patch-functions-3.c index 49b57a8..9e8eb52 100644 --- a/gcc/testsuite/gcc.target/i386/patch-functions-3.c +++ b/gcc/testsuite/gcc.target/i386/patch-functions-3.c @@ -1,4 +1,4 @@ -/* { dg-do run } */ +/* { dg-do compile } */ /* { dg-require-effective-target lp64 } */ /* { dg-options "-mpatch-functions-for-instrumentation --param function-patch-min-instructions=0" } */ @@ -8,11 +8,14 @@ /* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90" } } */ /* { dg-final { scan-assembler "ret(.*).byte\t0x90(.*).byte\t0x90" } } */ -void foo() { +__attribute__ ((noinline)) +void foo() +{ int x = 0; } -int main() { +int main() +{ foo(); return 0; } diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-4.c b/gcc/testsuite/gcc.target/i386/patch-functions-4.c index 5fcbd0f..7a031d7 100644 --- a/gcc/testsuite/gcc.target/i386/patch-functions-4.c +++ b/gcc/testsuite/gcc.target/i386/patch-functions-4.c @@ -1,4 +1,4 @@ -/* { dg-do run } */ +/* { dg-do compile } */ /* { dg-require-effective-target lp64 } */ /* { dg-options "-mpatch-functions-for-instrumentation -mpatch-functions-ignore-loops -mno-patch-functions-main-always" } */ @@ -8,12 +8,15 @@ /* { dg-final { scan-assembler-not ".byte\t0xeb,0x09(.*).byte\t0x90" } } */ /* { dg-final { scan-assembler-not "ret(.*).byte\t0x90(.*).byte\t0x90" } } */ -void foo() { +__attribute__ ((noinline)) +void foo() +{ int x = 0; while (++x); } -int main() { +int main() +{ foo(); return 0; } diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-5.c b/gcc/testsuite/gcc.target/i386/patch-functions-5.c index 1f9cccf..cd6a014 100644 --- a/gcc/testsuite/gcc.target/i386/patch-functions-5.c +++ b/gcc/testsuite/gcc.target/i386/patch-functions-5.c @@ -1,4 +1,4 @@ -/* { dg-do run } */ +/* { dg-do compile } */ /* { dg-require-effective-target lp64 } */ /* { dg-options "-mpatch-functions-for-instrumentation -mpatch-functions-ignore-loops --param function-patch-min-instructions=0" } */ @@ -8,12 +8,15 @@ /* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90" } } */ /* { dg-final { scan-assembler "ret(.*).byte\t0x90(.*).byte\t0x90" } } */ -void foo() { +__attribute__ ((noinline)) +void foo() +{ int x = 0; while (++x); } -int main() { +int main() +{ foo(); return 0; } diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-6.c b/gcc/testsuite/gcc.target/i386/patch-functions-6.c index 5bf844f..c1d6446 100644 --- a/gcc/testsuite/gcc.target/i386/patch-functions-6.c +++ b/gcc/testsuite/gcc.target/i386/patch-functions-6.c @@ -1,4 +1,4 @@ -/* { dg-do run } */ +/* { dg-do compile } */ /* { dg-require-effective-target lp64 } */ /* { dg-options "-mpatch-functions-for-instrumentation" } */ @@ -8,7 +8,8 @@ /* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90" } } */ /* { dg-final { scan-assembler "ret(.*).byte\t0x90(.*).byte\t0x90" } } */ -int main(int argc, char **argv) { +int main() +{ int x = 0; return 0; } diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-7.c b/gcc/testsuite/gcc.target/i386/patch-functions-7.c index e2c50a0..f625298 100644 --- a/gcc/testsuite/gcc.target/i386/patch-functions-7.c +++ b/gcc/testsuite/gcc.target/i386/patch-functions-7.c @@ -1,4 +1,4 @@ -/* { dg-do run } */ +/* { dg-do compile } */ /* { dg-require-effective-target lp64 } */ /* { dg-options "-mpatch-functions-for-instrumentation -mno-patch-functions-main-always" } */ @@ -8,7 +8,8 @@ /* { dg-final { scan-assembler-not ".byte\t0xeb,0x09(.*).byte\t0x90" } } */ /* { dg-final { scan-assembler-not "ret(.*).byte\t0x90(.*).byte\t0x90" } } */ -int main(int argc, char **argv) { +int main() +{ int x = 0; return 0; } diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-8.c b/gcc/testsuite/gcc.target/i386/patch-functions-8.c new file mode 100644 index 0000000..436379c --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/patch-functions-8.c @@ -0,0 +1,29 @@ +/* Verify -mpatch-functions-for-instrumentation works. */ +/* { dg-do run } */ +/* { dg-require-effective-target lp64 } */ + +/* -O2 forces a sibling call for foo from bar. */ +/* { dg-options "-O2 -mpatch-functions-for-instrumentation --param function-patch-min-instructions=0" } */ + +__attribute__ ((noinline)) +int foo() +{ + /* Dummy loop. */ + int x = 10; + int y = 100; + while (--x) + ++y; + return y; +} + +__attribute__ ((noinline)) +int bar() +{ + return foo(); +} + +int main() +{ + bar(); + return 0; +} diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-sibling-call.c b/gcc/testsuite/gcc.target/i386/patch-functions-sibling-call.c new file mode 100644 index 0000000..847a95c --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/patch-functions-sibling-call.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target lp64 } */ +/* -O2 forces a sibling call. */ +/* { dg-options "-O2 -mpatch-functions-for-instrumentation" } */ + +/* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90" } } */ + +/* Checks correct nop-bytes are generated just before a sibling call. */ +/* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90(.*)jmp" } } */ + +/* Not instrumented as function has no loop and is small. */ +__attribute__ ((noinline)) +int foo(int n) +{ + int x = 0; + return n + 10; +} + +__attribute__ ((noinline)) +int bar(int n) +{ + /* Dummy loop. */ + while (--n) + n = n * 2; + return foo(n); +}