Re: [PATCH] Added information about inline assembler in stack calculations (.su files)

[Torbjorn SVENSSON <torbjorn.svensson@st.com>]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

Hi!

Thanks for the feedback.
Attached is an updated patch where I switched to the NOP define instead.
I'm not sure if stack-usage-naked.c should be moved to gcc.dg-tree, or 
if it should skip using the nop.h file (it feels wrong to do #include 
"../../gcc.dg/nop.h" from within gcc.taget-tree).

Torbjörn

On 27/11/2018 19:40, Segher Boessenkool wrote:
> Hi!
>
> On Mon, Nov 26, 2018 at 02:02:49PM +0000, Torbjorn SVENSSON wrote:
>> Attached is a small patch that, in case of inline assembler code,
>> indicates that the function stack usage is uncertain due to inline
>> assembler.
>>
>> The test suite are using "nop" as an assembler instruction on all
>> targets, is this acceptable or is there a better way to test this?
> Maybe see testsuite/gcc.dg/nop.h ?
>
>
> Segher


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Added-information-about-inline-assembler-in-stack-ca.patch --]
[-- Type: text/x-patch; name="0001-Added-information-about-inline-assembler-in-stack-ca.patch", Size: 8004 bytes --]

From e8af10ab24c7e095604b16206c0bd544e8699b93 Mon Sep 17 00:00:00 2001
From: Niklas DAHLQUIST <niklas.dahlquist@st.com>
Date: Fri, 9 Nov 2018 18:48:34 +0100
Subject: [PATCH] Added information about inline assembler in stack
 calculations

The stack usage calculation in GCC ignores any possible stack usage that
any inline assembly might contribute, and this is not reflected in the
.su file currently.

Since inline assembly will add to the uncertainty of the actual stack
usage for a function, this should be shown in the .su file as well.

This changeset will add "ignoring_inline_assembly" to the .su-file "type"
information of functions containing inline assembly.

The resulting stack usage type for functions containing inline assembly
will be according to the following table:

Static | Dynamic     | Inline asm() | Resulting stack usage type
----------------------------------------------------------------
*      | 0           | False        | static
*      | 0           | True         | static,ignoring_inline_assembler
*      | 0 < x       | False        | dynamic
*      | 0 < x       | True         | dynamic,ignoring_inline_assembler
*      | 0 < x < MAX | False        | dynamic,bounded
*      | 0 < x < MAX | True         | dynamic,bounded,ignoring_inline_assembler

Added test case for ignore inline assembler in stack analysis files (.su)

Signed-off-by: Niklas DAHLQUIST <niklas.dahlquist@st.com>

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 38e27a50a1e..e61ddc3260b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -6456,7 +6456,7 @@ Warn if the stack usage of a function might exceed @var{byte-size}.
 The computation done to determine the stack usage is conservative.
 Any space allocated via @code{alloca}, variable-length arrays, or related
 constructs is included by the compiler when determining whether or not to
-issue a warning.
+issue a warning. @code{asm} statements are ignored when computing stack usage.
 
 The message is in keeping with the output of @option{-fstack-usage}.
 
diff --git a/gcc/function.c b/gcc/function.c
index 69523c1d723..197f80c0df3 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -1822,6 +1822,10 @@ instantiate_virtual_regs_in_insn (rtx_insn *insn)
 
   if (asm_noperands (PATTERN (insn)) >= 0)
     {
+       if (flag_stack_usage_info)
+	 {
+	   current_function_has_inline_assembler = 1;
+	 }
       if (!check_asm_operands (PATTERN (insn)))
 	{
 	  error_for_asm (insn, "impossible constraint in %<asm%>");
diff --git a/gcc/function.h b/gcc/function.h
index 7e59050e8a6..8c3ef49e866 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -208,11 +208,16 @@ struct GTY(()) stack_usage
   /* Nonzero if the amount of stack space allocated dynamically cannot
      be bounded at compile-time.  */
   unsigned int has_unbounded_dynamic_stack_size : 1;
+
+  /* NonZero if body contains asm statement (ignored in stack calculations) */
+  unsigned int has_inline_assembler: 1;
 };
 
 #define current_function_static_stack_size (cfun->su->static_stack_size)
 #define current_function_dynamic_stack_size (cfun->su->dynamic_stack_size)
 #define current_function_pushed_stack_size (cfun->su->pushed_stack_size)
+#define current_function_has_inline_assembler \
+  (cfun->su->has_inline_assembler)
 #define current_function_has_unbounded_dynamic_stack_size \
   (cfun->su->has_unbounded_dynamic_stack_size)
 #define current_function_allocates_dynamic_stack_space    \
diff --git a/gcc/testsuite/gcc.dg/stack-usage-3.c b/gcc/testsuite/gcc.dg/stack-usage-3.c
new file mode 100644
index 00000000000..ccb935f358e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/stack-usage-3.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-fstack-usage" } */
+/* nvptx doesn't have a reg allocator, and hence no stack usage data.  */
+/* { dg-skip-if "" { nvptx-*-* } { "*" } { "" } } */
+
+#include "nop.h"
+
+void foo()
+{
+    int i;
+    i = 1;
+}
+
+void bar()
+{
+    int i;
+    i = 1;
+    asm(NOP);
+}
+
+/* { dg-final { scan-stack-usage "foo\t\[1-9\]\[0-9\]*\tstatic" } } */
+/* { dg-final { scan-stack-usage "bar\t\[1-9\]\[0-9\]*\tstatic,ignoring_inline_asm" } } */
+/* { dg-final { cleanup-stack-usage } } */
+
diff --git a/gcc/testsuite/gcc.dg/stack-usage-4.c b/gcc/testsuite/gcc.dg/stack-usage-4.c
new file mode 100644
index 00000000000..c599729a8da
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/stack-usage-4.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-fstack-usage" } */
+/* nvptx doesn't have a reg allocator, and hence no stack usage data.  */
+/* { dg-skip-if "" { nvptx-*-* } { "*" } { "" } } */
+
+#include "nop.h"
+
+void foo(int size)
+{
+    int i;
+    int v[size];
+    i = 1;
+    v[0] = 1;
+}
+
+void bar(int size)
+{
+    int i;
+    int v[size];
+    i = 1;
+    v[0] = 1;
+    asm(NOP);
+}
+
+/* { dg-final { scan-stack-usage "foo\t\[1-9\]\[0-9\]*\tdynamic" } } */
+/* { dg-final { scan-stack-usage "bar\t\[1-9\]\[0-9\]*\tdynamic,ignoring_inline_asm" } } */
+/* { dg-final { cleanup-stack-usage } } */
diff --git a/gcc/testsuite/gcc.target/arm/stack-usage-naked.c b/gcc/testsuite/gcc.target/arm/stack-usage-naked.c
new file mode 100644
index 00000000000..72e7124c5e3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/stack-usage-naked.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options -fstack-usage } */
+
+
+void __attribute__((naked)) foo()
+{
+}
+
+void __attribute__((naked)) bar()
+{
+    asm("nop");
+}
+
+/* { dg-final { scan-stack-usage "foo\t0\tstatic" } } */
+/* { dg-final { scan-stack-usage "bar\t0\tstatic,ignoring_inline_asm" } } */
+/* { dg-final { cleanup-stack-usage } } */
diff --git a/gcc/toplev.c b/gcc/toplev.c
index ab20cd98969..14c930be9f3 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -937,11 +937,16 @@ void
 output_stack_usage (void)
 {
   static bool warning_issued = false;
-  enum stack_usage_kind_type { STATIC = 0, DYNAMIC, DYNAMIC_BOUNDED };
+  enum stack_usage_kind_type { STATIC = 0, DYNAMIC, DYNAMIC_BOUNDED,
+    STATIC_IGNORING_INLINE_ASM,DYNAMIC_IGNORING_INLINE_ASM,
+    DYNAMIC_BOUNDED_IGNORING_INLINE_ASM };
   const char *stack_usage_kind_str[] = {
     "static",
     "dynamic",
-    "dynamic,bounded"
+    "dynamic,bounded",
+    "static,ignoring_inline_asm",
+    "dynamic,ignoring_inline_asm",
+    "dynamic,bounded,ignoring_inline_asm"
   };
   HOST_WIDE_INT stack_usage = current_function_static_stack_size;
   enum stack_usage_kind_type stack_usage_kind;
@@ -990,6 +995,26 @@ output_stack_usage (void)
       stack_usage += current_function_dynamic_stack_size;
     }
 
+  /* Add info regarding inline assembler (not part of stack calculations) */
+  if (current_function_has_inline_assembler)
+    {
+	switch (stack_usage_kind)
+	  {
+	case STATIC:
+	  stack_usage_kind = STATIC_IGNORING_INLINE_ASM;
+	  break;
+	case DYNAMIC:
+	  stack_usage_kind = DYNAMIC_IGNORING_INLINE_ASM;
+	  break;
+	case DYNAMIC_BOUNDED:
+	  stack_usage_kind = DYNAMIC_BOUNDED_IGNORING_INLINE_ASM;
+	  break;
+	default:
+	  stack_usage_kind = STATIC_IGNORING_INLINE_ASM;
+	  break;
+	}
+    }
+
   if (stack_usage_file)
     {
       expanded_location loc
@@ -1031,11 +1056,13 @@ output_stack_usage (void)
     {
       const location_t loc = DECL_SOURCE_LOCATION (current_function_decl);
 
-      if (stack_usage_kind == DYNAMIC)
+      if (stack_usage_kind == DYNAMIC
+	  || stack_usage_kind == DYNAMIC_IGNORING_INLINE_ASM)
 	warning_at (loc, OPT_Wstack_usage_, "stack usage might be unbounded");
       else if (stack_usage > warn_stack_usage)
 	{
-	  if (stack_usage_kind == DYNAMIC_BOUNDED)
+	  if (stack_usage_kind == DYNAMIC_BOUNDED
+	      || stack_usage_kind == DYNAMIC_BOUNDED_IGNORING_INLINE_ASM)
 	    warning_at (loc,
 			OPT_Wstack_usage_, "stack usage might be %wu bytes",
 			stack_usage);
-- 
2.18.0