| Message ID | 20120321214558.5543B4314A@hchopra.mtv.corp.google.com |
|---|---|
| State | New |
| Headers | show |
Ok for google branches (main and 4_7). thanks, David On Wed, Mar 21, 2012 at 2:45 PM, Harshit Chopra <harshit@google.com> wrote: > 2012-03-21 Harshit Chopra <harshit@google.com> > > 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. > > 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); > +} > > -- > 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); +}
2012-03-21 Harshit Chopra <harshit@google.com> 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