diff mbox

Implement -fstack-usage option

Message ID 201008061941.55733.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou Aug. 6, 2010, 5:41 p.m. UTC 
> Most of this looks fairly sane.

Thanks.

> This last line ought to add to the dynamic stack size instead.

This makes sense indeed.

> (And you're over-estimating by 1 minimal-stack-alignment-unit.  ;-)

I think that this is mitigated by not adding the word for the new return 
address slot; that's what I was trying to say in the comment.  Here is a 
corrected version, attached.  I've also added a testcase for this, as well
as a generic one.


	* config/i386/i386.c (ix86_expand_prologue): Set stack usage info.
testsuite/
	* lib/gcc-dg.exp (cleanup-stack-usage): New procedure.
	* lib/scanasm.exp (scan-stack-usage): Likewise.
	(scan-stack-usage-not): Likewise.
	* gcc.dg/stack-usage-1.c: New test/
	* gcc.target/i386/stack-usage-realign.c: Likewise.

Comments

Eric Botcazou Aug. 30, 2010, 4:59 p.m. UTC | #1 
> I think that this is mitigated by not adding the word for the new return
> address slot; that's what I was trying to say in the comment.  Here is a
> corrected version, attached.  I've also added a testcase for this, as well
> as a generic one.
>
>
> 	* config/i386/i386.c (ix86_expand_prologue): Set stack usage info.
> testsuite/
> 	* lib/gcc-dg.exp (cleanup-stack-usage): New procedure.
> 	* lib/scanasm.exp (scan-stack-usage): Likewise.
> 	(scan-stack-usage-not): Likewise.
> 	* gcc.dg/stack-usage-1.c: New test/
> 	* gcc.target/i386/stack-usage-realign.c: Likewise.

Do you think this is good enough to be applied with the rest of the patch on 
the mainline?
Richard Henderson Aug. 30, 2010, 5:40 p.m. UTC | #2 
On 08/30/2010 09:59 AM, Eric Botcazou wrote:
>> I think that this is mitigated by not adding the word for the new return
>> address slot; that's what I was trying to say in the comment.  Here is a
>> corrected version, attached.  I've also added a testcase for this, as well
>> as a generic one.
>>
>>
>> 	* config/i386/i386.c (ix86_expand_prologue): Set stack usage info.
>> testsuite/
>> 	* lib/gcc-dg.exp (cleanup-stack-usage): New procedure.
>> 	* lib/scanasm.exp (scan-stack-usage): Likewise.
>> 	(scan-stack-usage-not): Likewise.
>> 	* gcc.dg/stack-usage-1.c: New test/
>> 	* gcc.target/i386/stack-usage-realign.c: Likewise.
> 
> Do you think this is good enough to be applied with the rest of the patch on 
> the mainline?
> 

Yes, this is fine.


r~
Jack Howarth Sept. 1, 2010, 8:31 p.m. UTC | #3 
On Fri, Aug 06, 2010 at 07:41:55PM +0200, Eric Botcazou wrote:
> > Most of this looks fairly sane.
> 
> Thanks.
> 
> > This last line ought to add to the dynamic stack size instead.
> 
> This makes sense indeed.
> 
> > (And you're over-estimating by 1 minimal-stack-alignment-unit.  ;-)
> 
> I think that this is mitigated by not adding the word for the new return 
> address slot; that's what I was trying to say in the comment.  Here is a 
> corrected version, attached.  I've also added a testcase for this, as well
> as a generic one.

Eric,
   Was this patch tested on x86_64-apple-darwin10 at adacore? Since the
patch was committed as r163660, we have been randomly failing three different
testcases...

FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os  (internal
compiler error)
UNRESOLVED: gcc.c-torture/execute/builtins/sprintf-chk.c execution,  -Os 
FAIL: gcc.dg/stack-usage-1.c scan-file foo\\t(256|264)\\tstatic
FAIL: gcc.target/i386/stack-usage-realign.c scan-file
main\\t48\\tdynamic,bounded

which Richard Henderson says looks like random memory corruption.
                  Jack

> 
> 
> 	* config/i386/i386.c (ix86_expand_prologue): Set stack usage info.
> testsuite/
> 	* lib/gcc-dg.exp (cleanup-stack-usage): New procedure.
> 	* lib/scanasm.exp (scan-stack-usage): Likewise.
> 	(scan-stack-usage-not): Likewise.
> 	* gcc.dg/stack-usage-1.c: New test/
> 	* gcc.target/i386/stack-usage-realign.c: Likewise.
> 
> 
> -- 
> Eric Botcazou

> Index: config/i386/i386.c
> ===================================================================
> --- config/i386/i386.c	(revision 162897)
> +++ config/i386/i386.c	(working copy)
> @@ -9588,6 +9588,29 @@ ix86_expand_prologue (void)
>      }
>    allocate = frame.stack_pointer_offset - m->fs.sp_offset;
>  
> +  if (flag_stack_usage)
> +    {
> +      /* We start to count from ARG_POINTER.  */
> +      HOST_WIDE_INT stack_size = frame.stack_pointer_offset;
> +
> +      /* If it was realigned, take into account the fake frame.  */
> +      if (stack_realign_drap)
> +	{
> +	  if (ix86_static_chain_on_stack)
> +	    stack_size += UNITS_PER_WORD;
> +
> +	  if (!call_used_regs[REGNO (crtl->drap_reg)])
> +	    stack_size += UNITS_PER_WORD;
> +
> +	  /* This over-estimates by 1 minimal-stack-alignment-unit but
> +	     mitigates that by counting in the new return address slot.  */
> +	  current_function_dynamic_stack_size
> +	    += crtl->stack_alignment_needed / BITS_PER_UNIT;
> +	}
> +
> +      current_function_static_stack_size = stack_size;
> +    }
> +
>    /* The stack has already been decremented by the instruction calling us
>       so we need to probe unconditionally to preserve the protection area.  */
>    if (flag_stack_check == STATIC_BUILTIN_STACK_CHECK)
> Index: testsuite/lib/gcc-dg.exp
> ===================================================================
> --- testsuite/lib/gcc-dg.exp	(revision 162897)
> +++ testsuite/lib/gcc-dg.exp	(working copy)
> @@ -1,5 +1,5 @@
> -#   Copyright (C) 1997, 1999, 2000, 2003, 2004, 2005, 2006, 2007, 2008, 2009
> -#   Free Software Foundation, Inc.
> +#   Copyright (C) 1997, 1999, 2000, 2003, 2004, 2005, 2006, 2007, 2008, 2009,
> +#   2010 Free Software Foundation, Inc.
>  
>  # This program is free software; you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> @@ -460,6 +460,11 @@ proc cleanup-ipa-dump { suffix } {
>    cleanup-dump "\[0-9\]\[0-9\]\[0-9\]i.$suffix"
>  }
>  
> +# Remove a stack usage file for the current test.
> +proc cleanup-stack-usage { args } {
> +  cleanup-dump "su"
> +}
> +
>  # Remove all dump files with the provided suffix.
>  proc cleanup-dump { suffix } {
>      # This assumes that we are three frames down from dg-test or some other
> Index: testsuite/lib/scanasm.exp
> ===================================================================
> --- testsuite/lib/scanasm.exp	(revision 162897)
> +++ testsuite/lib/scanasm.exp	(working copy)
> @@ -1,4 +1,5 @@
> -# Copyright (C) 2000, 2002, 2003, 2007, 2008 Free Software Foundation, Inc.
> +# Copyright (C) 2000, 2002, 2003, 2007, 2008, 2010
> +# Free Software Foundation, Inc.
>  
>  # This program is free software; you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> @@ -154,6 +155,28 @@ proc scan-file-not { output_file args }
>      dg-scan "scan-file-not" 0 $testcase $output_file $args
>  }
>  
> +# Look for a pattern in the .su file produced by the compiler.  See
> +# dg-scan for details.
> +
> +proc scan-stack-usage { args } {
> +    upvar 2 name testcase
> +    set testcase [lindex $testcase 0]
> +    set output_file "[file rootname [file tail $testcase]].su"
> +
> +    dg-scan "scan-file" 1 $testcase $output_file $args
> +}
> +
> +# Check that a pattern is not present in the .su file produced by the
> +# compiler.  See dg-scan for details.
> +
> +proc scan-stack-usage-not { args } {
> +    upvar 2 name testcase
> +    set testcase [lindex $testcase 0]
> +    set output_file "[file rootname [file tail $testcase]].su"
> +
> +    dg-scan "scan-file-not" 0 $testcase $output_file $args
> +}
> +
>  # Call pass if pattern is present given number of times, otherwise fail.
>  proc scan-assembler-times { args } {
>      if { [llength $args] < 2 } {

> /* { dg-do compile } */
> /* { dg-options "-fstack-usage" } */
> 
> /* This is aimed at testing basic support for -fstack-usage in the back-ends.
>    See the SPARC back-end for an example (grep flag_stack_usage in sparc.c).
>    Once it is implemented, adjust SIZE below so that the stack usage for the
>    function FOO is reported as 256 or 264 in the stack usage (.su) file.
>    Then check that this is the actual stack usage in the assembly file.  */
> 
> #if defined(__i386__)
> #  define SIZE 248
> #elif defined(__x86_64__)
> #  define SIZE 356
> #elif defined (__sparc__)
> #  if defined (__arch64__)
> #    define SIZE 76
> #  else
> #    define SIZE 160
> #  endif
> #elif defined(__hppa__)
> #  define SIZE 192
> #elif defined (__alpha__)
> #  define SIZE 240
> #elif defined (__ia64__)
> #  define SIZE 272
> #elif defined(__mips__)
> #  define SIZE 240
> #elif defined (__powerpc__) || defined (__PPC__) || defined (__ppc__) \
>       || defined (__POWERPC__) || defined (PPC) || defined (_IBMR2)
> #  define SIZE 240
> #else
> #  define SIZE 256
> #endif
> 
> int foo (void)
> {
>   char arr[SIZE];
>   arr[0] = 1;
>   return 0;
> }
> 
> /* { dg-final { scan-stack-usage "foo\t\(256|264\)\tstatic" } } */
> /* { dg-final { cleanup-stack-usage } } */

> /* { dg-do compile } */
> /* { dg-require-effective-target ilp32 } */
> /* { dg-options "-fstack-usage -msse2 -mforce-drap" } */
> 
> typedef int __attribute__((vector_size(16))) vec;
> 
> vec foo (vec v)
> {
>   return v;
> }
> 
> int main (void)
> {
>   vec V;
>   V = foo (V);
>   return 0;
> }
> 
> /* { dg-final { scan-stack-usage "main\t48\tdynamic,bounded" } } */
> /* { dg-final { cleanup-stack-usage } } */
Eric Botcazou Sept. 1, 2010, 9:22 p.m. UTC | #4 
> Since the patch was committed as r163660, we have been randomly failing
> three  different testcases...
>
> FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os 
> (internal compiler error)
> UNRESOLVED: gcc.c-torture/execute/builtins/sprintf-chk.c execution,  -Os
> FAIL: gcc.dg/stack-usage-1.c scan-file foo\\t(256|264)\\tstatic
> FAIL: gcc.target/i386/stack-usage-realign.c scan-file
> main\\t48\\tdynamic,bounded

Do the last 2 really pass sometimes?  They should _always_ fail on 32-bit 
Darwin as far as I can see...
Jack Howarth Sept. 1, 2010, 11:58 p.m. UTC | #5 
On Wed, Sep 01, 2010 at 11:22:26PM +0200, Eric Botcazou wrote:
> > Since the patch was committed as r163660, we have been randomly failing
> > three  different testcases...
> >
> > FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os 
> > (internal compiler error)
> > UNRESOLVED: gcc.c-torture/execute/builtins/sprintf-chk.c execution,  -Os
> > FAIL: gcc.dg/stack-usage-1.c scan-file foo\\t(256|264)\\tstatic
> > FAIL: gcc.target/i386/stack-usage-realign.c scan-file
> > main\\t48\\tdynamic,bounded
> 
> Do the last 2 really pass sometimes?  They should _always_ fail on 32-bit 
> Darwin as far as I can see...

Sorry, my mistake. The gcc.dg/stack-usage-1.c scan-file foo\\t(256|264)\\tstatic
and gcc.target/i386/stack-usage-realign.c scan-file main\\t48\\tdynamic,bounded
tests always compile and then fail on execution at -m32. Their code generation is
always consistent. It is the gcc.c-torture/execute/builtins/sprintf-chk.c
compilation which is exhibiting three behaviors...

1) consistent code
2) random differences in code
3) compiler ICE

My main concern is if this bug will spill over into production code generation.
             Jack

> 
> -- 
> Eric Botcazou
diff mbox

Patch

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 162897)
+++ config/i386/i386.c	(working copy)
@@ -9588,6 +9588,29 @@  ix86_expand_prologue (void)
     }
   allocate = frame.stack_pointer_offset - m->fs.sp_offset;
 
+  if (flag_stack_usage)
+    {
+      /* We start to count from ARG_POINTER.  */
+      HOST_WIDE_INT stack_size = frame.stack_pointer_offset;
+
+      /* If it was realigned, take into account the fake frame.  */
+      if (stack_realign_drap)
+	{
+	  if (ix86_static_chain_on_stack)
+	    stack_size += UNITS_PER_WORD;
+
+	  if (!call_used_regs[REGNO (crtl->drap_reg)])
+	    stack_size += UNITS_PER_WORD;
+
+	  /* This over-estimates by 1 minimal-stack-alignment-unit but
+	     mitigates that by counting in the new return address slot.  */
+	  current_function_dynamic_stack_size
+	    += crtl->stack_alignment_needed / BITS_PER_UNIT;
+	}
+
+      current_function_static_stack_size = stack_size;
+    }
+
   /* The stack has already been decremented by the instruction calling us
      so we need to probe unconditionally to preserve the protection area.  */
   if (flag_stack_check == STATIC_BUILTIN_STACK_CHECK)
Index: testsuite/lib/gcc-dg.exp
===================================================================
--- testsuite/lib/gcc-dg.exp	(revision 162897)
+++ testsuite/lib/gcc-dg.exp	(working copy)
@@ -1,5 +1,5 @@ 
-#   Copyright (C) 1997, 1999, 2000, 2003, 2004, 2005, 2006, 2007, 2008, 2009
-#   Free Software Foundation, Inc.
+#   Copyright (C) 1997, 1999, 2000, 2003, 2004, 2005, 2006, 2007, 2008, 2009,
+#   2010 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -460,6 +460,11 @@  proc cleanup-ipa-dump { suffix } {
   cleanup-dump "\[0-9\]\[0-9\]\[0-9\]i.$suffix"
 }
 
+# Remove a stack usage file for the current test.
+proc cleanup-stack-usage { args } {
+  cleanup-dump "su"
+}
+
 # Remove all dump files with the provided suffix.
 proc cleanup-dump { suffix } {
     # This assumes that we are three frames down from dg-test or some other
Index: testsuite/lib/scanasm.exp
===================================================================
--- testsuite/lib/scanasm.exp	(revision 162897)
+++ testsuite/lib/scanasm.exp	(working copy)
@@ -1,4 +1,5 @@ 
-# Copyright (C) 2000, 2002, 2003, 2007, 2008 Free Software Foundation, Inc.
+# Copyright (C) 2000, 2002, 2003, 2007, 2008, 2010
+# Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -154,6 +155,28 @@  proc scan-file-not { output_file args }
     dg-scan "scan-file-not" 0 $testcase $output_file $args
 }
 
+# Look for a pattern in the .su file produced by the compiler.  See
+# dg-scan for details.
+
+proc scan-stack-usage { args } {
+    upvar 2 name testcase
+    set testcase [lindex $testcase 0]
+    set output_file "[file rootname [file tail $testcase]].su"
+
+    dg-scan "scan-file" 1 $testcase $output_file $args
+}
+
+# Check that a pattern is not present in the .su file produced by the
+# compiler.  See dg-scan for details.
+
+proc scan-stack-usage-not { args } {
+    upvar 2 name testcase
+    set testcase [lindex $testcase 0]
+    set output_file "[file rootname [file tail $testcase]].su"
+
+    dg-scan "scan-file-not" 0 $testcase $output_file $args
+}
+
 # Call pass if pattern is present given number of times, otherwise fail.
 proc scan-assembler-times { args } {
     if { [llength $args] < 2 } {