public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] asm qualifiers (PR55681) and asm inline
@ 2018-12-02 16:38 Segher Boessenkool
  2018-12-02 16:39 ` [PATCH 1/2] asm qualifiers (PR55681) Segher Boessenkool
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Segher Boessenkool @ 2018-12-02 16:38 UTC (permalink / raw)
  To: gcc-patches
  Cc: jakub, Joseph Myers, jason, nathan, polacek, Segher Boessenkool

Hi!

v2, with the input from Joseph taken into account.

This is the same "asm inline" patch as before, but now preceded by a
patch that makes all orderings of volatile/goto/inline valid, all other
type qualifiers invalid, all repetitions of qualifiers invalid.

Tested on powerpc64-linux {-m32,-m64}.  Is this okay for trunk?


Segher


 gcc/c/c-parser.c                                | 91 +++++++++++++++---------
 gcc/c/c-tree.h                                  |  3 +-
 gcc/c/c-typeck.c                                |  3 +-
 gcc/cp/cp-tree.h                                |  2 +-
 gcc/cp/parser.c                                 | 92 +++++++++++++++++--------
 gcc/cp/pt.c                                     |  2 +-
 gcc/cp/semantics.c                              |  3 +-
 gcc/doc/extend.texi                             | 23 +++++--
 gcc/gimple-pretty-print.c                       |  2 +
 gcc/gimple.h                                    | 24 ++++++-
 gcc/gimplify.c                                  |  1 +
 gcc/ipa-icf-gimple.c                            |  3 +
 gcc/testsuite/c-c++-common/torture/asm-inline.c | 53 ++++++++++++++
 gcc/testsuite/gcc.dg/asm-qual-1.c               | 10 ++-
 gcc/testsuite/gcc.dg/asm-qual-2.c               | 46 +++++++++++++
 gcc/tree-core.h                                 |  3 +
 gcc/tree-inline.c                               |  3 +
 gcc/tree.h                                      |  3 +
 18 files changed, 294 insertions(+), 73 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/torture/asm-inline.c
 create mode 100644 gcc/testsuite/gcc.dg/asm-qual-2.c

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/2] asm qualifiers (PR55681)
  2018-12-02 16:38 [PATCH v2 0/2] asm qualifiers (PR55681) and asm inline Segher Boessenkool
@ 2018-12-02 16:39 ` Segher Boessenkool
  2018-12-03 22:20   ` Joseph Myers
  2018-12-05 21:47   ` Jason Merrill
  2018-12-02 16:40 ` [PATCH 2/2] asm inline Segher Boessenkool
  2018-12-06 18:11 ` [PATCH v2 0/2] asm qualifiers (PR55681) and " Segher Boessenkool
  2 siblings, 2 replies; 22+ messages in thread
From: Segher Boessenkool @ 2018-12-02 16:39 UTC (permalink / raw)
  To: gcc-patches
  Cc: jakub, Joseph Myers, jason, nathan, polacek, Segher Boessenkool

PR55681 observes that currently only one qualifier is allowed for
inline asm, so that e.g. "volatile asm" is allowed, "const asm" is also
okay (with a warning), but "const volatile asm" gives an error.  Also
"goto" has to be last.

This patch changes things so that only "asm-qualifiers" are allowed,
that is "volatile" and "goto", in any combination, in any order, but
without repetitions.


2018-12-02  Segher Boessenkool  <segher@kernel.crashing.org>

	PR inline-asm/55681
	* doc/extend.texi (Basic Asm): Update grammar.
	(Extended Asm): Update grammar.

gcc/c/
	PR inline-asm/55681
	* c-parser.c (c_parser_for_statement): Update grammar.  Allow any
	combination of volatile and goto, in any order, without repetitions.

gcc/cp/
	PR inline-asm/55681
	* parser.c (cp_parser_using_directive): Update grammar.  Allow any
	combination of volatile and goto, in any order, without repetitions.

gcc/testsuite/
	PR inline-asm/55681
	* gcc.dg/asm-qual-1.c: Test that "const" and "restrict" are refused.
	* gcc.dg/asm-qual-2.c: New test, test that asm-qualifiers are allowed
	in any order, but that duplicates are not allowed.

---
 gcc/c/c-parser.c                  | 74 +++++++++++++++++++++----------------
 gcc/cp/parser.c                   | 77 ++++++++++++++++++++++++++-------------
 gcc/doc/extend.texi               |  8 ++--
 gcc/testsuite/gcc.dg/asm-qual-1.c | 10 +++--
 gcc/testsuite/gcc.dg/asm-qual-2.c | 21 +++++++++++
 5 files changed, 127 insertions(+), 63 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/asm-qual-2.c

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index afc4071..e516f9a 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -6329,60 +6329,72 @@ c_parser_for_statement (c_parser *parser, bool ivdep, unsigned short unroll,
 }
 
 /* Parse an asm statement, a GNU extension.  This is a full-blown asm
-   statement with inputs, outputs, clobbers, and volatile tag
+   statement with inputs, outputs, clobbers, and volatile and goto tag
    allowed.
 
+   asm-qualifier:
+     volatile
+     goto
+
+   asm-qualifier-list:
+     asm-qualifier-list asm-qualifier
+     asm-qualifier
+
    asm-statement:
-     asm type-qualifier[opt] ( asm-argument ) ;
-     asm type-qualifier[opt] goto ( asm-goto-argument ) ;
+     asm asm-qualifier-list[opt] ( asm-argument ) ;
 
    asm-argument:
      asm-string-literal
      asm-string-literal : asm-operands[opt]
      asm-string-literal : asm-operands[opt] : asm-operands[opt]
-     asm-string-literal : asm-operands[opt] : asm-operands[opt] : asm-clobbers[opt]
-
-   asm-goto-argument:
+     asm-string-literal : asm-operands[opt] : asm-operands[opt] \
+       : asm-clobbers[opt]
      asm-string-literal : : asm-operands[opt] : asm-clobbers[opt] \
        : asm-goto-operands
 
-   Qualifiers other than volatile are accepted in the syntax but
-   warned for.  */
+   The form with asm-goto-operands is valid if and only if the
+   asm-qualifier-list contains goto, and is the only allowed form in that case.
+   Duplicate asm-qualifiers are not allowed.  */
 
 static tree
 c_parser_asm_statement (c_parser *parser)
 {
   tree quals, str, outputs, inputs, clobbers, labels, ret;
-  bool simple, is_goto;
+  bool simple, is_volatile, is_goto;
   location_t asm_loc = c_parser_peek_token (parser)->location;
   int section, nsections;
 
   gcc_assert (c_parser_next_token_is_keyword (parser, RID_ASM));
   c_parser_consume_token (parser);
-  if (c_parser_next_token_is_keyword (parser, RID_VOLATILE))
-    {
-      quals = c_parser_peek_token (parser)->value;
-      c_parser_consume_token (parser);
-    }
-  else if (c_parser_next_token_is_keyword (parser, RID_CONST)
-	   || c_parser_next_token_is_keyword (parser, RID_RESTRICT))
-    {
-      warning_at (c_parser_peek_token (parser)->location,
-		  0,
-		  "%E qualifier ignored on asm",
-		  c_parser_peek_token (parser)->value);
-      quals = NULL_TREE;
-      c_parser_consume_token (parser);
-    }
-  else
-    quals = NULL_TREE;
 
+  quals = NULL_TREE;
+  is_volatile = false;
   is_goto = false;
-  if (c_parser_next_token_is_keyword (parser, RID_GOTO))
-    {
-      c_parser_consume_token (parser);
-      is_goto = true;
-    }
+  for (bool done = false; !done; )
+    switch (c_parser_peek_token (parser)->keyword)
+      {
+      case RID_VOLATILE:
+	if (!is_volatile)
+	  {
+	    is_volatile = true;
+	    quals = c_parser_peek_token (parser)->value;
+	    c_parser_consume_token (parser);
+	  }
+	else
+	  done = true;
+	break;
+      case RID_GOTO:
+	if (!is_goto)
+	  {
+	    is_goto = true;
+	    c_parser_consume_token (parser);
+	  }
+	else
+	  done = true;
+	break;
+      default:
+	done = true;
+      }
 
   /* ??? Follow the C++ parser rather than using the
      lex_untranslated_string kludge.  */
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 3a98ed9..9c43683 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -19537,22 +19537,34 @@ cp_parser_using_directive (cp_parser* parser)
 
 /* Parse an asm-definition.
 
+  asm-qualifier:
+    volatile
+    goto
+
+  asm-qualifier-list:
+    asm-qualifier
+    asm-qualifier-list asm-qualifier
+
    asm-definition:
      asm ( string-literal ) ;
 
    GNU Extension:
 
    asm-definition:
-     asm volatile [opt] ( string-literal ) ;
-     asm volatile [opt] ( string-literal : asm-operand-list [opt] ) ;
-     asm volatile [opt] ( string-literal : asm-operand-list [opt]
-			  : asm-operand-list [opt] ) ;
-     asm volatile [opt] ( string-literal : asm-operand-list [opt]
-			  : asm-operand-list [opt]
+     asm asm-qualifier-list [opt] ( string-literal ) ;
+     asm asm-qualifier-list [opt] ( string-literal : asm-operand-list [opt] ) ;
+     asm asm-qualifier-list [opt] ( string-literal : asm-operand-list [opt]
+				    : asm-operand-list [opt] ) ;
+     asm asm-qualifier-list [opt] ( string-literal : asm-operand-list [opt]
+				    : asm-operand-list [opt]
 			  : asm-clobber-list [opt] ) ;
-     asm volatile [opt] goto ( string-literal : : asm-operand-list [opt]
-			       : asm-clobber-list [opt]
-			       : asm-goto-list ) ;  */
+     asm asm-qualifier-list [opt] ( string-literal : : asm-operand-list [opt]
+				    : asm-clobber-list [opt]
+				    : asm-goto-list ) ;
+
+  The form with asm-goto-list is valid if and only if the asm-qualifier-list
+  contains goto, and is the only allowed form in that case.  No duplicates are
+  allowed in an asm-qualifier-list.  */
 
 static void
 cp_parser_asm_definition (cp_parser* parser)
@@ -19581,23 +19593,36 @@ cp_parser_asm_definition (cp_parser* parser)
     }
 
   /* See if the next token is `volatile'.  */
-  if (cp_parser_allow_gnu_extensions_p (parser)
-      && cp_lexer_next_token_is_keyword (parser->lexer, RID_VOLATILE))
-    {
-      /* Remember that we saw the `volatile' keyword.  */
-      volatile_p = true;
-      /* Consume the token.  */
-      cp_lexer_consume_token (parser->lexer);
-    }
-  if (cp_parser_allow_gnu_extensions_p (parser)
-      && parser->in_function_body
-      && cp_lexer_next_token_is_keyword (parser->lexer, RID_GOTO))
-    {
-      /* Remember that we saw the `goto' keyword.  */
-      goto_p = true;
-      /* Consume the token.  */
-      cp_lexer_consume_token (parser->lexer);
-    }
+  if (cp_parser_allow_gnu_extensions_p (parser))
+    for (bool done = false; !done ; )
+      switch (cp_lexer_peek_token (parser->lexer)->keyword)
+	{
+	case RID_VOLATILE:
+	  if (!volatile_p)
+	    {
+	      /* Remember that we saw the `volatile' keyword.  */
+	      volatile_p = true;
+	      /* Consume the token.  */
+	      cp_lexer_consume_token (parser->lexer);
+	    }
+	  else
+	    done = true;
+	  break;
+	case RID_GOTO:
+	  if (!goto_p && parser->in_function_body)
+	    {
+	      /* Remember that we saw the `goto' keyword.  */
+	      goto_p = true;
+	      /* Consume the token.  */
+	      cp_lexer_consume_token (parser->lexer);
+	    }
+	  else
+	    done = true;
+	  break;
+	default:
+	  done = true;
+	}
+
   /* Look for the opening `('.  */
   if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
     return;
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 4e8be5b..2791f25 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -8369,7 +8369,7 @@ for a C symbol, or to place a C variable in a specific register.
 A basic @code{asm} statement has the following syntax:
 
 @example
-asm @r{[} volatile @r{]} ( @var{AssemblerInstructions} )
+asm @var{asm-qualifiers} ( @var{AssemblerInstructions} )
 @end example
 
 The @code{asm} keyword is a GNU extension.
@@ -8497,17 +8497,19 @@ Extended @code{asm} syntax uses colons (@samp{:}) to delimit
 the operand parameters after the assembler template:
 
 @example
-asm @r{[}volatile@r{]} ( @var{AssemblerTemplate} 
+asm @var{asm-qualifiers} ( @var{AssemblerTemplate} 
                  : @var{OutputOperands} 
                  @r{[} : @var{InputOperands}
                  @r{[} : @var{Clobbers} @r{]} @r{]})
 
-asm @r{[}volatile@r{]} goto ( @var{AssemblerTemplate} 
+asm @var{asm-qualifiers} ( @var{AssemblerTemplate} 
                       : 
                       : @var{InputOperands}
                       : @var{Clobbers}
                       : @var{GotoLabels})
 @end example
+where in the last form, @var{asm-qualifiers} contains @code{goto} (and in the
+first form, not).
 
 The @code{asm} keyword is a GNU extension.
 When writing code that can be compiled with @option{-ansi} and the
diff --git a/gcc/testsuite/gcc.dg/asm-qual-1.c b/gcc/testsuite/gcc.dg/asm-qual-1.c
index 5ec9a29..cb37283 100644
--- a/gcc/testsuite/gcc.dg/asm-qual-1.c
+++ b/gcc/testsuite/gcc.dg/asm-qual-1.c
@@ -1,4 +1,4 @@
-/* Test that qualifiers other than volatile are ignored on asm.  */
+/* Test that qualifiers other than volatile are disallowed on asm.  */
 /* Origin: Joseph Myers <joseph@codesourcery.com> */
 /* { dg-do compile } */
 /* { dg-options "-std=gnu99" } */
@@ -7,6 +7,10 @@ void
 f (void)
 {
   asm volatile ("");
-  asm const (""); /* { dg-warning "const qualifier ignored on asm" } */
-  asm restrict (""); /* { dg-warning "restrict qualifier ignored on asm" } */
+
+  asm const (""); /* { dg-error {expected '\(' before 'const'} } */
+  /* { dg-error {expected identifier} {} {target *-*-*} .-1 } */
+
+  asm restrict (""); /* { dg-error {expected '\(' before 'restrict'} } */
+  /* { dg-error {expected identifier} {} {target *-*-*} .-1 } */
 }
diff --git a/gcc/testsuite/gcc.dg/asm-qual-2.c b/gcc/testsuite/gcc.dg/asm-qual-2.c
new file mode 100644
index 0000000..37df2ad
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asm-qual-2.c
@@ -0,0 +1,21 @@
+/* Test that qualifiers on asm are allowed in any order.  */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" } */
+
+void
+f (void)
+{
+  asm volatile goto ("" :::: lab);
+  asm goto volatile ("" :::: lab);
+
+  /* Duplicates are not allowed.  */
+  asm goto volatile volatile ("" :::: lab);  /* { dg-error "" } */
+  asm volatile goto volatile ("" :::: lab);  /* { dg-error "" } */
+  asm volatile volatile goto ("" :::: lab);  /* { dg-error "" } */
+  asm goto goto volatile ("" :::: lab);  /* { dg-error "" } */
+  asm goto volatile goto ("" :::: lab);  /* { dg-error "" } */
+  asm volatile goto goto ("" :::: lab);  /* { dg-error "" } */
+
+lab:
+  ;
+}
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/2] asm inline
  2018-12-02 16:38 [PATCH v2 0/2] asm qualifiers (PR55681) and asm inline Segher Boessenkool
  2018-12-02 16:39 ` [PATCH 1/2] asm qualifiers (PR55681) Segher Boessenkool
@ 2018-12-02 16:40 ` Segher Boessenkool
  2018-12-02 17:23   ` Marc Glisse
  2018-12-04 15:31   ` Richard Sandiford
  2018-12-06 18:11 ` [PATCH v2 0/2] asm qualifiers (PR55681) and " Segher Boessenkool
  2 siblings, 2 replies; 22+ messages in thread
From: Segher Boessenkool @ 2018-12-02 16:40 UTC (permalink / raw)
  To: gcc-patches
  Cc: jakub, Joseph Myers, jason, nathan, polacek, Segher Boessenkool

The Linux kernel people want a feature that makes GCC pretend some
inline assembler code is tiny (while it would think it is huge), so
that such code will be inlined essentially always instead of
essentially never.

This patch lets you say "asm inline" instead of just "asm", with the
result that that inline assembler is always counted as minimum cost
for inlining.  It implements this for C and C++, making "inline"
another asm-qualifier (supplementing "volatile" and "goto").


2018-12-02  Segher Boessenkool  <segher@kernel.crashing.org>

        * doc/extend.texi (Using Assembly Language with C): Document asm inline.
        (Size of an asm): Fix typo.  Document asm inline.
        * gimple-pretty-print.c (dump_gimple_asm): Handle asm inline.
        * gimple.h (enum gf_mask): Add GF_ASM_INLINE.
        (gimple_asm_set_volatile): Fix typo.
        (gimple_asm_inline_p): New.
        (gimple_asm_set_inline): New.
        * gimplify.c (gimplify_asm_expr): Propagate the asm inline flag from
        tree to gimple.
        * ipa-icf-gimple.c (func_checker::compare_gimple_asm): Compare the
        gimple_asm_inline_p flag, too.
        * tree-core.h (tree_base): Document that protected_flag is ASM_INLINE_P
        in an ASM_EXPR.
        * tree-inline.c (estimate_num_insns): If gimple_asm_inline_p return
        a minimum size for an asm.
        * tree.h (ASM_INLINE_P): New.

gcc/c/
        * c-parser.c (c_parser_asm_statement): Detect the inline keyword
        after asm.  Pass a flag for it to build_asm_expr.
        * c-tree.h (build_asm_expr): Update declaration.
        * c-typeck.c (build_asm_stmt): Add is_inline parameter.  Use it to
        set ASM_INLINE_P.

gcc/cp/
        * cp-tree.h (finish_asm_stmt): Update declaration.
        * parser.c (cp_parser_asm_definition): Detect the inline keyword
        after asm.  Pass a flag for it to finish_asm_stmt.
        * pt.c (tsubst_expr): Pass the ASM_INLINE_P flag to finish_asm_stmt.
        * semantics.c (finish_asm_stmt): Add inline_p parameter.  Use it to
        set ASM_INLINE_P.

gcc/testsuite/
        * c-c++-common/torture/asm-inline.c: New testcase.
	* gcc.dg/asm-qual-2.c: Test asm inline, too.

---
 gcc/c/c-parser.c                                | 21 ++++++++--
 gcc/c/c-tree.h                                  |  3 +-
 gcc/c/c-typeck.c                                |  3 +-
 gcc/cp/cp-tree.h                                |  2 +-
 gcc/cp/parser.c                                 | 15 ++++++-
 gcc/cp/pt.c                                     |  2 +-
 gcc/cp/semantics.c                              |  3 +-
 gcc/doc/extend.texi                             | 15 ++++++-
 gcc/gimple-pretty-print.c                       |  2 +
 gcc/gimple.h                                    | 24 ++++++++++-
 gcc/gimplify.c                                  |  1 +
 gcc/ipa-icf-gimple.c                            |  3 ++
 gcc/testsuite/c-c++-common/torture/asm-inline.c | 53 +++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/asm-qual-2.c               | 25 ++++++++++++
 gcc/tree-core.h                                 |  3 ++
 gcc/tree-inline.c                               |  3 ++
 gcc/tree.h                                      |  3 ++
 17 files changed, 169 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/torture/asm-inline.c

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index e516f9a..921363d 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -6329,11 +6329,12 @@ c_parser_for_statement (c_parser *parser, bool ivdep, unsigned short unroll,
 }
 
 /* Parse an asm statement, a GNU extension.  This is a full-blown asm
-   statement with inputs, outputs, clobbers, and volatile and goto tag
-   allowed.
+   statement with inputs, outputs, clobbers, and volatile, inline, and goto
+   tags allowed.
 
    asm-qualifier:
      volatile
+     inline
      goto
 
    asm-qualifier-list:
@@ -6360,7 +6361,7 @@ static tree
 c_parser_asm_statement (c_parser *parser)
 {
   tree quals, str, outputs, inputs, clobbers, labels, ret;
-  bool simple, is_volatile, is_goto;
+  bool simple, is_volatile, is_inline, is_goto;
   location_t asm_loc = c_parser_peek_token (parser)->location;
   int section, nsections;
 
@@ -6369,6 +6370,7 @@ c_parser_asm_statement (c_parser *parser)
 
   quals = NULL_TREE;
   is_volatile = false;
+  is_inline = false;
   is_goto = false;
   for (bool done = false; !done; )
     switch (c_parser_peek_token (parser)->keyword)
@@ -6383,6 +6385,16 @@ c_parser_asm_statement (c_parser *parser)
 	else
 	  done = true;
 	break;
+      case RID_INLINE:
+	if (!is_inline)
+	  {
+	    is_inline = true;
+	    quals = c_parser_peek_token (parser)->value;
+	    c_parser_consume_token (parser);
+	  }
+	else
+	  done = true;
+	break;
       case RID_GOTO:
 	if (!is_goto)
 	  {
@@ -6471,7 +6483,8 @@ c_parser_asm_statement (c_parser *parser)
     c_parser_skip_to_end_of_block_or_statement (parser);
 
   ret = build_asm_stmt (quals, build_asm_expr (asm_loc, str, outputs, inputs,
-					       clobbers, labels, simple));
+					       clobbers, labels, simple,
+					       is_inline));
 
  error:
   parser->lex_untranslated_string = false;
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 5ed2f48..f08a8fc 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -677,7 +677,8 @@ extern tree build_compound_literal (location_t, tree, tree, bool,
 extern void check_compound_literal_type (location_t, struct c_type_name *);
 extern tree c_start_case (location_t, location_t, tree, bool);
 extern void c_finish_case (tree, tree);
-extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool);
+extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool,
+			    bool);
 extern tree build_asm_stmt (tree, tree);
 extern int c_types_compatible_p (tree, tree);
 extern tree c_begin_compound_stmt (bool);
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 81c520a..9b572d7 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -10315,7 +10315,7 @@ build_asm_stmt (tree cv_qualifier, tree args)
    are subtly different.  We use a ASM_EXPR node to represent this.  */
 tree
 build_asm_expr (location_t loc, tree string, tree outputs, tree inputs,
-		tree clobbers, tree labels, bool simple)
+		tree clobbers, tree labels, bool simple, bool is_inline)
 {
   tree tail;
   tree args;
@@ -10433,6 +10433,7 @@ build_asm_expr (location_t loc, tree string, tree outputs, tree inputs,
      as volatile.  */
   ASM_INPUT_P (args) = simple;
   ASM_VOLATILE_P (args) = (noutputs == 0);
+  ASM_INLINE_P (args) = is_inline;
 
   return args;
 }
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 111a123..26acf31 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6996,7 +6996,7 @@ extern tree begin_compound_stmt			(unsigned int);
 
 extern void finish_compound_stmt		(tree);
 extern tree finish_asm_stmt			(int, tree, tree, tree, tree,
-						 tree);
+						 tree, bool);
 extern tree finish_label_stmt			(tree);
 extern void finish_label_decl			(tree);
 extern cp_expr finish_parenthesized_expr	(cp_expr);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 9c43683..64d073e 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -19539,6 +19539,7 @@ cp_parser_using_directive (cp_parser* parser)
 
   asm-qualifier:
     volatile
+    inline
     goto
 
   asm-qualifier-list:
@@ -19579,6 +19580,7 @@ cp_parser_asm_definition (cp_parser* parser)
   bool extended_p = false;
   bool invalid_inputs_p = false;
   bool invalid_outputs_p = false;
+  bool inline_p = false;
   bool goto_p = false;
   required_token missing = RT_NONE;
 
@@ -19608,6 +19610,17 @@ cp_parser_asm_definition (cp_parser* parser)
 	  else
 	    done = true;
 	  break;
+	case RID_INLINE:
+	  if (!inline_p && parser->in_function_body)
+	    {
+	      /* Remember that we saw the `inline' keyword.  */
+	      inline_p = true;
+	      /* Consume the token.  */
+	      cp_lexer_consume_token (parser->lexer);
+	    }
+	  else
+	    done = true;
+	  break;
 	case RID_GOTO:
 	  if (!goto_p && parser->in_function_body)
 	    {
@@ -19749,7 +19762,7 @@ cp_parser_asm_definition (cp_parser* parser)
       if (parser->in_function_body)
 	{
 	  asm_stmt = finish_asm_stmt (volatile_p, string, outputs,
-				      inputs, clobbers, labels);
+				      inputs, clobbers, labels, inline_p);
 	  /* If the extended syntax was not used, mark the ASM_EXPR.  */
 	  if (!extended_p)
 	    {
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index a0d899f..7d846a0 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -17226,7 +17226,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 	tree labels = tsubst_copy_asm_operands (ASM_LABELS (t), args,
 						complain, in_decl);
 	tmp = finish_asm_stmt (ASM_VOLATILE_P (t), string, outputs, inputs,
-			       clobbers, labels);
+			       clobbers, labels, ASM_INLINE_P (t));
 	tree asm_expr = tmp;
 	if (TREE_CODE (asm_expr) == CLEANUP_POINT_EXPR)
 	  asm_expr = TREE_OPERAND (asm_expr, 0);
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 733c42f..87d54b1 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -1486,7 +1486,7 @@ finish_compound_stmt (tree stmt)
 
 tree
 finish_asm_stmt (int volatile_p, tree string, tree output_operands,
-		 tree input_operands, tree clobbers, tree labels)
+		 tree input_operands, tree clobbers, tree labels, bool inline_p)
 {
   tree r;
   tree t;
@@ -1640,6 +1640,7 @@ finish_asm_stmt (int volatile_p, tree string, tree output_operands,
 		  output_operands, input_operands,
 		  clobbers, labels);
   ASM_VOLATILE_P (r) = volatile_p || noutputs == 0;
+  ASM_INLINE_P (r) = inline_p;
   r = maybe_cleanup_point_expr_void (r);
   return add_stmt (r);
 }
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 2791f25..cebbfc7 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -8382,6 +8382,10 @@ various @option{-std} options, use @code{__asm__} instead of
 @item volatile
 The optional @code{volatile} qualifier has no effect. 
 All basic @code{asm} blocks are implicitly volatile.
+
+@item inline
+If you use the @code{inline} qualifier, then for inlining purposes the size
+of the asm is taken as the smallest size possible (@pxref{Size of an asm}).
 @end table
 
 @subsubheading Parameters
@@ -8525,6 +8529,10 @@ values to produce output values. However, your @code{asm} statements may
 also produce side effects. If so, you may need to use the @code{volatile} 
 qualifier to disable certain optimizations. @xref{Volatile}.
 
+@item inline
+If you use the @code{inline} qualifier, then for inlining purposes the size
+of the asm is taken as the smallest size possible (@pxref{Size of an asm}).
+
 @item goto
 This qualifier informs the compiler that the @code{asm} statement may 
 perform a jump to one of the labels listed in the @var{GotoLabels}.
@@ -9983,7 +9991,7 @@ does this by counting the number of instructions in the pattern of the
 @code{asm} and multiplying that by the length of the longest
 instruction supported by that processor.  (When working out the number
 of instructions, it assumes that any occurrence of a newline or of
-whatever statement separator character is supported by the assembler --
+whatever statement separator character is supported by the assembler ---
 typically @samp{;} --- indicates the end of an instruction.)
 
 Normally, GCC's estimate is adequate to ensure that correct
@@ -9994,6 +10002,11 @@ space in the object file than is needed for a single instruction.
 If this happens then the assembler may produce a diagnostic saying that
 a label is unreachable.
 
+@cindex @code{asm inline}
+This size is also used for inlining decisions.  If you use @code{asm inline}
+instead of just @code{asm}, then for inlining purposes the size of the asm
+is taken as the minimum size, ignoring how many instructions GCC thinks it is.
+
 @node Alternate Keywords
 @section Alternate Keywords
 @cindex alternate keywords
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index 276e579..51c0fb3 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -2048,6 +2048,8 @@ dump_gimple_asm (pretty_printer *buffer, gasm *gs, int spc, dump_flags_t flags)
       pp_string (buffer, "__asm__");
       if (gimple_asm_volatile_p (gs))
 	pp_string (buffer, " __volatile__");
+      if (gimple_asm_inline_p (gs))
+	pp_string (buffer, " __inline__");
       if (gimple_asm_nlabels (gs))
 	pp_string (buffer, " goto");
       pp_string (buffer, "(\"");
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 9853521..0d0ee16 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -137,6 +137,7 @@ enum gimple_rhs_class
 enum gf_mask {
     GF_ASM_INPUT		= 1 << 0,
     GF_ASM_VOLATILE		= 1 << 1,
+    GF_ASM_INLINE		= 1 << 2,
     GF_CALL_FROM_THUNK		= 1 << 0,
     GF_CALL_RETURN_SLOT_OPT	= 1 << 1,
     GF_CALL_TAILCALL		= 1 << 2,
@@ -3920,7 +3921,7 @@ gimple_asm_volatile_p (const gasm *asm_stmt)
 }
 
 
-/* If VOLATLE_P is true, mark asm statement ASM_STMT as volatile.  */
+/* If VOLATILE_P is true, mark asm statement ASM_STMT as volatile.  */
 
 static inline void
 gimple_asm_set_volatile (gasm *asm_stmt, bool volatile_p)
@@ -3932,6 +3933,27 @@ gimple_asm_set_volatile (gasm *asm_stmt, bool volatile_p)
 }
 
 
+/* Return true ASM_STMT ASM_STMT is an asm statement marked inline.  */
+
+static inline bool
+gimple_asm_inline_p (const gasm *asm_stmt)
+{
+  return (asm_stmt->subcode & GF_ASM_INLINE) != 0;
+}
+
+
+/* If INLINE_P is true, mark asm statement ASM_STMT as inline.  */
+
+static inline void
+gimple_asm_set_inline (gasm *asm_stmt, bool inline_p)
+{
+  if (inline_p)
+    asm_stmt->subcode |= GF_ASM_INLINE;
+  else
+    asm_stmt->subcode &= ~GF_ASM_INLINE;
+}
+
+
 /* If INPUT_P is true, mark asm ASM_STMT as an ASM_INPUT.  */
 
 static inline void
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 40fbaa2..0e95ff2 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -6357,6 +6357,7 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 
       gimple_asm_set_volatile (stmt, ASM_VOLATILE_P (expr) || noutputs == 0);
       gimple_asm_set_input (stmt, ASM_INPUT_P (expr));
+      gimple_asm_set_inline (stmt, ASM_INLINE_P (expr));
 
       gimplify_seq_add_stmt (pre_p, stmt);
     }
diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index ba39ea3..5361139 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -993,6 +993,9 @@ func_checker::compare_gimple_asm (const gasm *g1, const gasm *g2)
   if (gimple_asm_input_p (g1) != gimple_asm_input_p (g2))
     return false;
 
+  if (gimple_asm_inline_p (g1) != gimple_asm_inline_p (g2))
+    return false;
+
   if (gimple_asm_ninputs (g1) != gimple_asm_ninputs (g2))
     return false;
 
diff --git a/gcc/testsuite/c-c++-common/torture/asm-inline.c b/gcc/testsuite/c-c++-common/torture/asm-inline.c
new file mode 100644
index 0000000..dea8965
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/torture/asm-inline.c
@@ -0,0 +1,53 @@
+/* { dg-do compile } */
+/* -O0 does no inlining, and -O3 does it too aggressively for this test:  */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O3" } { "" } }
+/* The normal asm is not inlined:  */
+/* { dg-final { scan-assembler-times "w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w" 2 } } */
+/* But the asm inline is inlined:  */
+/* { dg-final { scan-assembler-times "x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x" 8 } } */
+
+static void f(void)
+{
+  asm ("w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\n"
+       "w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw");
+}
+
+int f0(void) { f(); return 0; }
+int f1(void) { f(); return 1; }
+int f2(void) { f(); return 2; }
+int f3(void) { f(); return 3; }
+
+static void fg(void)
+{
+  asm goto("w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\n"
+	   "w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw" :::: q);
+  q: ;
+}
+
+int fg0(void) { fg(); return 0; }
+int fg1(void) { fg(); return 1; }
+int fg2(void) { fg(); return 2; }
+int fg3(void) { fg(); return 3; }
+
+static void g(void)
+{
+  asm inline("x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\n"
+	     "x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx");
+}
+
+int g0(void) { g(); return 0; }
+int g1(void) { g(); return 1; }
+int g2(void) { g(); return 2; }
+int g3(void) { g(); return 3; }
+
+static void gg(void)
+{
+  asm inline goto("x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\n"
+		  "x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx" :::: q);
+  q: ;
+}
+
+int gg0(void) { gg(); return 0; }
+int gg1(void) { gg(); return 1; }
+int gg2(void) { gg(); return 2; }
+int gg3(void) { gg(); return 3; }
diff --git a/gcc/testsuite/gcc.dg/asm-qual-2.c b/gcc/testsuite/gcc.dg/asm-qual-2.c
index 37df2ad..79135c3 100644
--- a/gcc/testsuite/gcc.dg/asm-qual-2.c
+++ b/gcc/testsuite/gcc.dg/asm-qual-2.c
@@ -6,7 +6,18 @@ void
 f (void)
 {
   asm volatile goto ("" :::: lab);
+  asm volatile inline ("" :::);
+  asm inline volatile ("" :::);
+  asm inline goto ("" :::: lab);
   asm goto volatile ("" :::: lab);
+  asm goto inline ("" :::: lab);
+
+  asm volatile inline goto ("" :::: lab);
+  asm volatile goto inline ("" :::: lab);
+  asm inline volatile goto ("" :::: lab);
+  asm inline goto volatile ("" :::: lab);
+  asm goto volatile inline ("" :::: lab);
+  asm goto inline volatile ("" :::: lab);
 
   /* Duplicates are not allowed.  */
   asm goto volatile volatile ("" :::: lab);  /* { dg-error "" } */
@@ -16,6 +27,20 @@ f (void)
   asm goto volatile goto ("" :::: lab);  /* { dg-error "" } */
   asm volatile goto goto ("" :::: lab);  /* { dg-error "" } */
 
+  asm inline volatile volatile ("" :::);  /* { dg-error "" } */
+  asm volatile inline volatile ("" :::);  /* { dg-error "" } */
+  asm volatile volatile inline ("" :::);  /* { dg-error "" } */
+  asm inline inline volatile ("" :::);  /* { dg-error "" } */
+  asm inline volatile inline ("" :::);  /* { dg-error "" } */
+  asm volatile inline inline ("" :::);  /* { dg-error "" } */
+
+  asm goto inline inline ("" :::: lab);  /* { dg-error "" } */
+  asm inline goto inline ("" :::: lab);  /* { dg-error "" } */
+  asm inline inline goto ("" :::: lab);  /* { dg-error "" } */
+  asm goto goto inline ("" :::: lab);  /* { dg-error "" } */
+  asm goto inline goto ("" :::: lab);  /* { dg-error "" } */
+  asm inline goto goto ("" :::: lab);  /* { dg-error "" } */
+
 lab:
   ;
 }
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index cec95e7..e7646af 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1193,6 +1193,9 @@ struct GTY(()) tree_base {
        OMP_CLAUSE_LINEAR_VARIABLE_STRIDE in
 	   OMP_CLAUSE_LINEAR
 
+       ASM_INLINE_P in
+	   ASM_EXPR
+
    side_effects_flag:
 
        TREE_SIDE_EFFECTS in
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 5aa782b..7e9ed99 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -4109,6 +4109,9 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
 	   with very long asm statements.  */
 	if (count > 1000)
 	  count = 1000;
+	/* If this asm is asm inline, count anything as minimum size.  */
+	if (gimple_asm_inline_p (as_a <gasm *> (stmt)))
+	  count = !!count;
 	return MAX (1, count);
       }
 
diff --git a/gcc/tree.h b/gcc/tree.h
index 0767ee8..76147f3 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1245,6 +1245,9 @@ extern tree maybe_wrap_with_location (tree, location_t);
    ASM_OPERAND with no operands.  */
 #define ASM_INPUT_P(NODE) (ASM_EXPR_CHECK (NODE)->base.static_flag)
 #define ASM_VOLATILE_P(NODE) (ASM_EXPR_CHECK (NODE)->base.public_flag)
+/* Nonzero if we want to consider this asm as minimum length and cost
+   for inlining decisions.  */
+#define ASM_INLINE_P(NODE) (ASM_EXPR_CHECK (NODE)->base.protected_flag)
 
 /* COND_EXPR accessors.  */
 #define COND_EXPR_COND(NODE)	(TREE_OPERAND (COND_EXPR_CHECK (NODE), 0))
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-12-02 16:40 ` [PATCH 2/2] asm inline Segher Boessenkool
@ 2018-12-02 17:23   ` Marc Glisse
  2018-12-02 17:45     ` Segher Boessenkool
  2018-12-04 15:31   ` Richard Sandiford
  1 sibling, 1 reply; 22+ messages in thread
From: Marc Glisse @ 2018-12-02 17:23 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: gcc-patches, jakub, Joseph Myers, jason, nathan, polacek

On Sun, 2 Dec 2018, Segher Boessenkool wrote:

> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index 5aa782b..7e9ed99 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -4109,6 +4109,9 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
> 	   with very long asm statements.  */
> 	if (count > 1000)
> 	  count = 1000;
> +	/* If this asm is asm inline, count anything as minimum size.  */
> +	if (gimple_asm_inline_p (as_a <gasm *> (stmt)))
> +	  count = !!count;
> 	return MAX (1, count);
>       }

Hello,

what is the point of !!count when we take the max with 1 on the very
next line? Is it in prevision of a time when we may remove the MAX? (sorry 
if this was covered in previous iterations)

By the way, not related to the patch, but I wonder why we cannot have a
cost of 0. My main use of inline asm is as an optimization barrier:
asm("":"+gx"(local_var))
possibly marked volatile to prevent more optimizations. I certainly
expect it to generate exactly 0 instruction in most cases. Although if I 
am not careful it could easily generate moves from x87 to sse/memory for 
instance. I guess a minimal cost is safer and doesn't affect decisions too 
badly.

-- 
Marc Glisse

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-12-02 17:23   ` Marc Glisse
@ 2018-12-02 17:45     ` Segher Boessenkool
  2018-12-02 18:38       ` Marc Glisse
  0 siblings, 1 reply; 22+ messages in thread
From: Segher Boessenkool @ 2018-12-02 17:45 UTC (permalink / raw)
  To: gcc-patches; +Cc: jakub, Joseph Myers, jason, nathan, polacek

Hi!

On Sun, Dec 02, 2018 at 06:23:23PM +0100, Marc Glisse wrote:
> On Sun, 2 Dec 2018, Segher Boessenkool wrote:
> 
> >diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> >index 5aa782b..7e9ed99 100644
> >--- a/gcc/tree-inline.c
> >+++ b/gcc/tree-inline.c
> >@@ -4109,6 +4109,9 @@ estimate_num_insns (gimple *stmt, eni_weights 
> >*weights)
> >	   with very long asm statements.  */
> >	if (count > 1000)
> >	  count = 1000;
> >+	/* If this asm is asm inline, count anything as minimum size.  */
> >+	if (gimple_asm_inline_p (as_a <gasm *> (stmt)))
> >+	  count = !!count;
> >	return MAX (1, count);
> >      }
> 
> what is the point of !!count when we take the max with 1 on the very
> next line? Is it in prevision of a time when we may remove the MAX? (sorry 
> if this was covered in previous iterations)
> 
> By the way, not related to the patch, but I wonder why we cannot have a
> cost of 0.

That exactly is the point :-)  My code still works if you remove that MAX
expression, as hopefully we will some day.  Right now GCC will of course
optimise it to "count = 1;", but writing it like that doesn't make the
intent clear.

I think this workaround is here because otherwise we get infinite recursion
in the inliner, but that of course should be fixed, not worked around.
Ideally.  If anyone ever has time for it.  :-)

> My main use of inline asm is as an optimization barrier:
> asm("":"+gx"(local_var))
> possibly marked volatile to prevent more optimizations. I certainly
> expect it to generate exactly 0 instruction in most cases. Although if I 
> am not careful it could easily generate moves from x87 to sse/memory for 
> instance. I guess a minimal cost is safer and doesn't affect decisions too 
> badly.

This is only for inlining; GCC _does_ know such asms are cost 0, and uses
that for all other purposes.

("gx", btw?  Is that a typo?  Or, on what target is "x" useful here?)


Segher

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-12-02 17:45     ` Segher Boessenkool
@ 2018-12-02 18:38       ` Marc Glisse
  0 siblings, 0 replies; 22+ messages in thread
From: Marc Glisse @ 2018-12-02 18:38 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: gcc-patches, jakub, Joseph Myers, jason, nathan, polacek

On Sun, 2 Dec 2018, Segher Boessenkool wrote:

>> what is the point of !!count when we take the max with 1 on the very
>> next line? Is it in prevision of a time when we may remove the MAX? (sorry
>> if this was covered in previous iterations)
>>
>> By the way, not related to the patch, but I wonder why we cannot have a
>> cost of 0.
>
> That exactly is the point :-)  My code still works if you remove that MAX
> expression, as hopefully we will some day.  Right now GCC will of course
> optimise it to "count = 1;", but writing it like that doesn't make the
> intent clear.
>
> I think this workaround is here because otherwise we get infinite recursion
> in the inliner, but that of course should be fixed, not worked around.
> Ideally.  If anyone ever has time for it.  :-)

Thanks.

Note that I may have 2 or 3 such asm per floating point operation, which
could be enough to skew inlining decisions. On the other hand, the
protected operations can never be optimized (that's the whole point of
the asm), which is a reason not to inline too much. I never really had a
problem, I was just curious.

>> My main use of inline asm is as an optimization barrier:
>> asm("":"+gx"(local_var))
>> possibly marked volatile to prevent more optimizations. I certainly
>> expect it to generate exactly 0 instruction in most cases. Although if I
>> am not careful it could easily generate moves from x87 to sse/memory for
>> instance. I guess a minimal cost is safer and doesn't affect decisions too
>> badly.
>
> This is only for inlining; GCC _does_ know such asms are cost 0, and uses
> that for all other purposes.
>
> ("gx", btw?  Is that a typo?  Or, on what target is "x" useful here?)

(context: -frounding-math doesn't work, so I have to protect double values)
On x64_64, "x" is for SSE registers, and those are not included in "g". 
When "gx" fails (ICE on old gcc, bad codegen with llvm) I use "mx" ("gx" 
does not really bring much compared to "mx" for a double) or even just "x" 
since "m" doesn't quite work as I would like. I have this rather sad code 
(slightly edited) where no 2 platforms use the same letter:

# if defined __SSE2_MATH__
   asm volatile ("" : "+gx"(x) );
# elif (defined __i386__ || defined __x86_64__)
   asm volatile ("" : "+mt"(x) );
# elif (defined __VFP_FP__ && !defined __SOFTFP__) || defined __aarch64__
   asm volatile ("" : "+gw"(x) );
# elif defined __powerpc__ || defined __POWERPC__
   asm volatile ("" : "+gd"(x) );
# elif defined __sparc
   asm volatile ("" : "+ge"(x) );
# elif defined __ia64
   asm volatile ("" : "+gf"(x) );
# else
   asm volatile ("" : "+g"(x) );
# endif

-- 
Marc Glisse

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] asm qualifiers (PR55681)
  2018-12-02 16:39 ` [PATCH 1/2] asm qualifiers (PR55681) Segher Boessenkool
@ 2018-12-03 22:20   ` Joseph Myers
  2018-12-05 21:47   ` Jason Merrill
  1 sibling, 0 replies; 22+ messages in thread
From: Joseph Myers @ 2018-12-03 22:20 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches, jakub, jason, nathan, polacek

On Sun, 2 Dec 2018, Segher Boessenkool wrote:

> PR55681 observes that currently only one qualifier is allowed for
> inline asm, so that e.g. "volatile asm" is allowed, "const asm" is also
> okay (with a warning), but "const volatile asm" gives an error.  Also
> "goto" has to be last.
> 
> This patch changes things so that only "asm-qualifiers" are allowed,
> that is "volatile" and "goto", in any combination, in any order, but
> without repetitions.

The C front-end changes are OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-12-02 16:40 ` [PATCH 2/2] asm inline Segher Boessenkool
  2018-12-02 17:23   ` Marc Glisse
@ 2018-12-04 15:31   ` Richard Sandiford
  2018-12-06  3:03     ` Segher Boessenkool
  1 sibling, 1 reply; 22+ messages in thread
From: Richard Sandiford @ 2018-12-04 15:31 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: gcc-patches, jakub, Joseph Myers, jason, nathan, polacek

Segher Boessenkool <segher@kernel.crashing.org> writes:
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 81c520a..9b572d7 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -10315,7 +10315,7 @@ build_asm_stmt (tree cv_qualifier, tree args)
>     are subtly different.  We use a ASM_EXPR node to represent this.  */
>  tree
>  build_asm_expr (location_t loc, tree string, tree outputs, tree inputs,
> -		tree clobbers, tree labels, bool simple)
> +		tree clobbers, tree labels, bool simple, bool is_inline)
>  {
>    tree tail;
>    tree args;

Function comment doesn't document the new parameter.

> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index 733c42f..87d54b1 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -1486,7 +1486,7 @@ finish_compound_stmt (tree stmt)
>  
>  tree
>  finish_asm_stmt (int volatile_p, tree string, tree output_operands,
> -		 tree input_operands, tree clobbers, tree labels)
> +		 tree input_operands, tree clobbers, tree labels, bool inline_p)
>  {
>    tree r;
>    tree t;

Same here.

> @@ -19608,6 +19610,17 @@ cp_parser_asm_definition (cp_parser* parser)
>  	  else
>  	    done = true;
>  	  break;
> +	case RID_INLINE:
> +	  if (!inline_p && parser->in_function_body)
> +	    {
> +	      /* Remember that we saw the `inline' keyword.  */
> +	      inline_p = true;
> +	      /* Consume the token.  */
> +	      cp_lexer_consume_token (parser->lexer);
> +	    }
> +	  else
> +	    done = true;
> +	  break;
>  	case RID_GOTO:
>  	  if (!goto_p && parser->in_function_body)
>  	    {

Hmm, so we allow top-level "asm volatile" in C++ but not C?

Probably worth having tests to show that we (intentionally) don't
allow top-level "asm inline".

> @@ -1640,6 +1640,7 @@ finish_asm_stmt (int volatile_p, tree string, tree output_operands,
>  		  output_operands, input_operands,
>  		  clobbers, labels);
>    ASM_VOLATILE_P (r) = volatile_p || noutputs == 0;
> +  ASM_INLINE_P (r) = inline_p;
>    r = maybe_cleanup_point_expr_void (r);
>    return add_stmt (r);
>  }
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 2791f25..cebbfc7 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -8382,6 +8382,10 @@ various @option{-std} options, use @code{__asm__} instead of
>  @item volatile
>  The optional @code{volatile} qualifier has no effect. 
>  All basic @code{asm} blocks are implicitly volatile.
> +
> +@item inline
> +If you use the @code{inline} qualifier, then for inlining purposes the size
> +of the asm is taken as the smallest size possible (@pxref{Size of an asm}).
>  @end table
>  
>  @subsubheading Parameters

You need to update the syntax above too, which currently reads:

@example
asm @r{[} volatile @r{]} ( @var{AssemblerInstructions} )
@end example

Same for the equivalent extended asm docs.

> @@ -3932,6 +3933,27 @@ gimple_asm_set_volatile (gasm *asm_stmt, bool volatile_p)
>  }
>  
>  
> +/* Return true ASM_STMT ASM_STMT is an asm statement marked inline.  */
> +
> +static inline bool
> +gimple_asm_inline_p (const gasm *asm_stmt)
> +{
> +  return (asm_stmt->subcode & GF_ASM_INLINE) != 0;
> +}

Return true if ASM_STMT is ...

(Or "Return true if asm statement ASM_STMT is marked inline", since gasm
forces it to be an asm statement.)

> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index 5aa782b..7e9ed99 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -4109,6 +4109,9 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
>  	   with very long asm statements.  */
>  	if (count > 1000)
>  	  count = 1000;
> +	/* If this asm is asm inline, count anything as minimum size.  */
> +	if (gimple_asm_inline_p (as_a <gasm *> (stmt)))
> +	  count = !!count;

FWIW, since Marc found it confusing too... I think MIN (count, 1) would
show the intent more clearerly.  But that's just personal preference,
no need to change.

OK with those changes, thanks.

Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] asm qualifiers (PR55681)
  2018-12-02 16:39 ` [PATCH 1/2] asm qualifiers (PR55681) Segher Boessenkool
  2018-12-03 22:20   ` Joseph Myers
@ 2018-12-05 21:47   ` Jason Merrill
  2018-12-05 23:02     ` Segher Boessenkool
  1 sibling, 1 reply; 22+ messages in thread
From: Jason Merrill @ 2018-12-05 21:47 UTC (permalink / raw)
  To: Segher Boessenkool, gcc-patches; +Cc: jakub, Joseph Myers, nathan, polacek

On 12/2/18 11:38 AM, Segher Boessenkool wrote:
> PR55681 observes that currently only one qualifier is allowed for
> inline asm, so that e.g. "volatile asm" is allowed, "const asm" is also
> okay (with a warning), but "const volatile asm" gives an error.  Also
> "goto" has to be last.
> 
> This patch changes things so that only "asm-qualifiers" are allowed,
> that is "volatile" and "goto", in any combination, in any order, but
> without repetitions.
> 
> 
> 2018-12-02  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> 	PR inline-asm/55681
> 	* doc/extend.texi (Basic Asm): Update grammar.
> 	(Extended Asm): Update grammar.
> 
> gcc/c/
> 	PR inline-asm/55681
> 	* c-parser.c (c_parser_for_statement): Update grammar.  Allow any
> 	combination of volatile and goto, in any order, without repetitions.
> 
> gcc/cp/
> 	PR inline-asm/55681
> 	* parser.c (cp_parser_using_directive): Update grammar.  Allow any
> 	combination of volatile and goto, in any order, without repetitions.

You don't actually change cp_parser_using_directive, despite what diff 
says: you're changing cp_parser_asm_definition.

> +    for (bool done = false; !done ; )
> +      switch (cp_lexer_peek_token (parser->lexer)->keyword)
> +	{
> +	case RID_VOLATILE:
> +	  if (!volatile_p)
> +	    {
> +	      /* Remember that we saw the `volatile' keyword.  */
> +	      volatile_p = true;
> +	      /* Consume the token.  */
> +	      cp_lexer_consume_token (parser->lexer);
> +	    }
> +	  else
> +	    done = true;
> +	  break;
> +	case RID_GOTO:
> +	  if (!goto_p && parser->in_function_body)
> +	    {
> +	      /* Remember that we saw the `goto' keyword.  */
> +	      goto_p = true;
> +	      /* Consume the token.  */
> +	      cp_lexer_consume_token (parser->lexer);
> +	    }
> +	  else
> +	    done = true;
> +	  break;
> +	default:
> +	  done = true;
> +	}

An arguably simpler alternative to using the 'done' variable would be to 
'break' out of the loop after the switch, and have the consume_token 
cases explicitly 'continue'.

We also might remember the old tokens and give a more helpful error 
message in the case of duplicate keywords.

But I won't insist on either of these, the C++ changes are OK as-is.

Jason

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] asm qualifiers (PR55681)
  2018-12-05 21:47   ` Jason Merrill
@ 2018-12-05 23:02     ` Segher Boessenkool
  0 siblings, 0 replies; 22+ messages in thread
From: Segher Boessenkool @ 2018-12-05 23:02 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, jakub, Joseph Myers, nathan, polacek

Hi!

On Wed, Dec 05, 2018 at 04:47:37PM -0500, Jason Merrill wrote:
> On 12/2/18 11:38 AM, Segher Boessenkool wrote:
> >PR55681 observes that currently only one qualifier is allowed for
> >inline asm, so that e.g. "volatile asm" is allowed, "const asm" is also
> >okay (with a warning), but "const volatile asm" gives an error.  Also
> >"goto" has to be last.
> >
> >This patch changes things so that only "asm-qualifiers" are allowed,
> >that is "volatile" and "goto", in any combination, in any order, but
> >without repetitions.
> >
> >
> >2018-12-02  Segher Boessenkool  <segher@kernel.crashing.org>
> >
> >	PR inline-asm/55681
> >	* doc/extend.texi (Basic Asm): Update grammar.
> >	(Extended Asm): Update grammar.
> >
> >gcc/c/
> >	PR inline-asm/55681
> >	* c-parser.c (c_parser_for_statement): Update grammar.  Allow any
> >	combination of volatile and goto, in any order, without repetitions.
> >
> >gcc/cp/
> >	PR inline-asm/55681
> >	* parser.c (cp_parser_using_directive): Update grammar.  Allow any
> >	combination of volatile and goto, in any order, without repetitions.
> 
> You don't actually change cp_parser_using_directive, despite what diff 
> says: you're changing cp_parser_asm_definition.

I trust diff too much, sigh.

> >+    for (bool done = false; !done ; )
> >+      switch (cp_lexer_peek_token (parser->lexer)->keyword)
> >+	{
> >+	case RID_VOLATILE:
> >+	  if (!volatile_p)
> >+	    {
> >+	      /* Remember that we saw the `volatile' keyword.  */
> >+	      volatile_p = true;
> >+	      /* Consume the token.  */
> >+	      cp_lexer_consume_token (parser->lexer);
> >+	    }
> >+	  else
> >+	    done = true;
> >+	  break;
> >+	case RID_GOTO:
> >+	  if (!goto_p && parser->in_function_body)
> >+	    {
> >+	      /* Remember that we saw the `goto' keyword.  */
> >+	      goto_p = true;
> >+	      /* Consume the token.  */
> >+	      cp_lexer_consume_token (parser->lexer);
> >+	    }
> >+	  else
> >+	    done = true;
> >+	  break;
> >+	default:
> >+	  done = true;
> >+	}
> 
> An arguably simpler alternative to using the 'done' variable would be to 
> 'break' out of the loop after the switch, and have the consume_token 
> cases explicitly 'continue'.

Yeah, that is neater, continue only deals with the loop.  Nice.

> We also might remember the old tokens and give a more helpful error 
> message in the case of duplicate keywords.
> 
> But I won't insist on either of these, the C++ changes are OK as-is.

I'll commit it like this then, and work on the improvements afterwards
(they also apply to the C frontend).

Thanks for the review!


Segher

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-12-04 15:31   ` Richard Sandiford
@ 2018-12-06  3:03     ` Segher Boessenkool
  0 siblings, 0 replies; 22+ messages in thread
From: Segher Boessenkool @ 2018-12-06  3:03 UTC (permalink / raw)
  To: gcc-patches, jakub, Joseph Myers, jason, nathan, polacek,
	richard.sandiford

Hi!

On Tue, Dec 04, 2018 at 03:30:47PM +0000, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:

> Hmm, so we allow top-level "asm volatile" in C++ but not C?

Apparently.  Evert top-level asm is effectively volatile, so this doesn't
really matter; but should we try to resolve the difference?

> Probably worth having tests to show that we (intentionally) don't
> allow top-level "asm inline".

Okay, I'll do this separately.

> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -8382,6 +8382,10 @@ various @option{-std} options, use @code{__asm__} instead of
> >  @item volatile
> >  The optional @code{volatile} qualifier has no effect. 
> >  All basic @code{asm} blocks are implicitly volatile.
> > +
> > +@item inline
> > +If you use the @code{inline} qualifier, then for inlining purposes the size
> > +of the asm is taken as the smallest size possible (@pxref{Size of an asm}).
> >  @end table
> >  
> >  @subsubheading Parameters
> 
> You need to update the syntax above too, which currently reads:
> 
> @example
> asm @r{[} volatile @r{]} ( @var{AssemblerInstructions} )
> @end example

That was modified in patch 1?

@example
asm @var{asm-qualifiers} ( @var{AssemblerInstructions} )
@end example

> > +/* Return true ASM_STMT ASM_STMT is an asm statement marked inline.  */
> > +
> > +static inline bool
> > +gimple_asm_inline_p (const gasm *asm_stmt)
> > +{
> > +  return (asm_stmt->subcode & GF_ASM_INLINE) != 0;
> > +}
> 
> Return true if ASM_STMT is ...

Heh, I copied this from the "volatile" one above.  I'll fix that one, too.

> (Or "Return true if asm statement ASM_STMT is marked inline", since gasm
> forces it to be an asm statement.)

Yup, same deal.

> > diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> > index 5aa782b..7e9ed99 100644
> > --- a/gcc/tree-inline.c
> > +++ b/gcc/tree-inline.c
> > @@ -4109,6 +4109,9 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
> >  	   with very long asm statements.  */
> >  	if (count > 1000)
> >  	  count = 1000;
> > +	/* If this asm is asm inline, count anything as minimum size.  */
> > +	if (gimple_asm_inline_p (as_a <gasm *> (stmt)))
> > +	  count = !!count;
> 
> FWIW, since Marc found it confusing too... I think MIN (count, 1) would
> show the intent more clearerly.  But that's just personal preference,
> no need to change.

MIN is a nice way to say it, thanks!  Better than "!!" or "!= 0" here.

Thanks for the review, I'm committing it with the changes above (later
today, after a final bootstrap),


Segher

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 0/2] asm qualifiers (PR55681) and asm inline
  2018-12-02 16:38 [PATCH v2 0/2] asm qualifiers (PR55681) and asm inline Segher Boessenkool
  2018-12-02 16:39 ` [PATCH 1/2] asm qualifiers (PR55681) Segher Boessenkool
  2018-12-02 16:40 ` [PATCH 2/2] asm inline Segher Boessenkool
@ 2018-12-06 18:11 ` Segher Boessenkool
  2018-12-06 18:15   ` Jakub Jelinek
  2 siblings, 1 reply; 22+ messages in thread
From: Segher Boessenkool @ 2018-12-06 18:11 UTC (permalink / raw)
  To: gcc-patches; +Cc: jakub, Joseph Myers, jason, nathan, polacek

Hi all,

On Sun, Dec 02, 2018 at 04:38:16PM +0000, Segher Boessenkool wrote:
> v2, with the input from Joseph taken into account.
> 
> This is the same "asm inline" patch as before, but now preceded by a
> patch that makes all orderings of volatile/goto/inline valid, all other
> type qualifiers invalid, all repetitions of qualifiers invalid.

Committed now, with everyone's suggestions addressed.

Is this okay for backport to 8?  Maybe 7?  After a week or so, of course.
This will help the Linux people to use it sooner.


Segher

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 0/2] asm qualifiers (PR55681) and asm inline
  2018-12-06 18:11 ` [PATCH v2 0/2] asm qualifiers (PR55681) and " Segher Boessenkool
@ 2018-12-06 18:15   ` Jakub Jelinek
  2018-12-06 18:19     ` Joseph Myers
  2018-12-06 18:54     ` Segher Boessenkool
  0 siblings, 2 replies; 22+ messages in thread
From: Jakub Jelinek @ 2018-12-06 18:15 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches, Joseph Myers, jason, nathan, polacek

On Thu, Dec 06, 2018 at 12:10:56PM -0600, Segher Boessenkool wrote:
> On Sun, Dec 02, 2018 at 04:38:16PM +0000, Segher Boessenkool wrote:
> > v2, with the input from Joseph taken into account.
> > 
> > This is the same "asm inline" patch as before, but now preceded by a
> > patch that makes all orderings of volatile/goto/inline valid, all other
> > type qualifiers invalid, all repetitions of qualifiers invalid.
> 
> Committed now, with everyone's suggestions addressed.
> 
> Is this okay for backport to 8?  Maybe 7?  After a week or so, of course.
> This will help the Linux people to use it sooner.

Not sure if in the backport we shouldn't keep accepting with warning like
before const asm and not do the changes of accepting in any order except
perhaps for the inline keyword in there?

	Jakub

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 0/2] asm qualifiers (PR55681) and asm inline
  2018-12-06 18:15   ` Jakub Jelinek
@ 2018-12-06 18:19     ` Joseph Myers
  2018-12-06 18:54     ` Segher Boessenkool
  1 sibling, 0 replies; 22+ messages in thread
From: Joseph Myers @ 2018-12-06 18:19 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Segher Boessenkool, gcc-patches, jason, nathan, polacek

On Thu, 6 Dec 2018, Jakub Jelinek wrote:

> Not sure if in the backport we shouldn't keep accepting with warning like
> before const asm and not do the changes of accepting in any order except
> perhaps for the inline keyword in there?

Indeed, I think it's best to keep accepting const and restrict in asm for 
any backports, to avoid breaking any existing code with those constructs 
with a release branch change.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 0/2] asm qualifiers (PR55681) and asm inline
  2018-12-06 18:15   ` Jakub Jelinek
  2018-12-06 18:19     ` Joseph Myers
@ 2018-12-06 18:54     ` Segher Boessenkool
  1 sibling, 0 replies; 22+ messages in thread
From: Segher Boessenkool @ 2018-12-06 18:54 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Joseph Myers, jason, nathan, polacek

On Thu, Dec 06, 2018 at 07:15:20PM +0100, Jakub Jelinek wrote:
> On Thu, Dec 06, 2018 at 12:10:56PM -0600, Segher Boessenkool wrote:
> > On Sun, Dec 02, 2018 at 04:38:16PM +0000, Segher Boessenkool wrote:
> > > v2, with the input from Joseph taken into account.
> > > 
> > > This is the same "asm inline" patch as before, but now preceded by a
> > > patch that makes all orderings of volatile/goto/inline valid, all other
> > > type qualifiers invalid, all repetitions of qualifiers invalid.
> > 
> > Committed now, with everyone's suggestions addressed.
> > 
> > Is this okay for backport to 8?  Maybe 7?  After a week or so, of course.
> > This will help the Linux people to use it sooner.
> 
> Not sure if in the backport we shouldn't keep accepting with warning like
> before const asm and not do the changes of accepting in any order except
> perhaps for the inline keyword in there?

Okay, I'll edit the const (etc.) back in.  The "any order" is easier to
keep this way I think (and doesn't change anything for code that was
already accepted).


Segher

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-10-30 18:41 ` [PATCH 2/2] asm inline Segher Boessenkool
  2018-10-30 18:58   ` Marek Polacek
  2018-11-11 21:33   ` Martin Sebor
@ 2018-11-30 13:14   ` Richard Biener
  2 siblings, 0 replies; 22+ messages in thread
From: Richard Biener @ 2018-11-30 13:14 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: GCC Patches, Jakub Jelinek

On Tue, Oct 30, 2018 at 6:31 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> The Linux kernel people want a feature that makes GCC pretend some
> inline assembler code is tiny (while it would think it is huge), so
> that such code will be inlined essentially always instead of
> essentially never.
>
> This patch lets you say "asm inline" instead of just "asm", with the
> result that that inline assembler is always counted as minimum cost
> for inlining.  It implements this for C and C++.

The middle-end and documentation changes are OK.  I suppose the FE changes
are as well in case [2/2] is approved.

Thanks,
Richard.

>
> 2018-10-30  Segher Boessenkool  <segher@kernel.crashing.org>
>
>         * doc/extend.texi (Using Assembly Language with C): Document asm inline.
>         (Size of an asm): Fix typo.  Document asm inline.
>         * gimple-pretty-print.c (dump_gimple_asm): Handle asm inline.
>         * gimple.h (enum gf_mask): Add GF_ASM_INLINE.
>         (gimple_asm_set_volatile): Fix typo.
>         * gimple_asm_inline_p: New.
>         * gimple_asm_set_inline: New.
>         * gimplify.c (gimplify_asm_expr): Propagate the asm inline flag from
>         tree to gimple.
>         * ipa-icf-gimple.c (func_checker::compare_gimple_asm): Compare the
>         gimple_asm_inline_p flag, too.
>         * tree-core.h (tree_base): Document that protected_flag is ASM_INLINE_P
>         in an ASM_EXPR.
>         * tree-inline.c (estimate_num_insns): If gimple_asm_inline_p return
>         a minimum size for an asm.
>         * tree.h (ASM_INLINE_P): New.
>
> gcc/c/
>         * c-parser.c (c_parser_asm_statement): Detect the inline keyword
>         after asm.  Pass a flag for it to build_asm_expr.
>         * c-tree.h (build_asm_expr): Update declaration.
>         * c-typeck.c (build_asm_stmt): Add is_inline parameter.  Use it to
>         set ASM_INLINE_P.
>
> gcc/cp/
>         * cp-tree.h (finish_asm_stmt): Update declaration.
>         * parser.c (cp_parser_asm_definition): Detect the inline keyword
>         after asm.  Pass a flag for it to finish_asm_stmt.
>         * pt.c (tsubst_expr): Pass the ASM_INLINE_P flag to finish_asm_stmt.
>         * semantics.c (finish_asm_stmt): Add inline_p parameter.  Use it to
>         set ASM_INLINE_P.
>
> gcc/testsuite/
>         * c-c++-common/torture/asm-inline.c: New testcase.
>
> ---
>  gcc/c/c-parser.c                                | 15 +++++--
>  gcc/c/c-tree.h                                  |  3 +-
>  gcc/c/c-typeck.c                                |  3 +-
>  gcc/cp/cp-tree.h                                |  2 +-
>  gcc/cp/parser.c                                 | 15 ++++++-
>  gcc/cp/pt.c                                     |  2 +-
>  gcc/cp/semantics.c                              |  3 +-
>  gcc/doc/extend.texi                             | 10 ++++-
>  gcc/gimple-pretty-print.c                       |  2 +
>  gcc/gimple.h                                    | 24 ++++++++++-
>  gcc/gimplify.c                                  |  1 +
>  gcc/ipa-icf-gimple.c                            |  3 ++
>  gcc/testsuite/c-c++-common/torture/asm-inline.c | 53 +++++++++++++++++++++++++
>  gcc/tree-core.h                                 |  3 ++
>  gcc/tree-inline.c                               |  3 ++
>  gcc/tree.h                                      |  3 ++
>  16 files changed, 133 insertions(+), 12 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/torture/asm-inline.c
>
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index ce9921e..b28b712 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -6283,11 +6283,12 @@ c_parser_for_statement (c_parser *parser, bool ivdep, unsigned short unroll,
>  }
>
>  /* Parse an asm statement, a GNU extension.  This is a full-blown asm
> -   statement with inputs, outputs, clobbers, and volatile and goto tag
> -   allowed.
> +   statement with inputs, outputs, clobbers, and volatile, inline, and goto
> +   tags allowed.
>
>     asm-qualifier:
>       type-qualifier
> +     inline
>       goto
>
>     asm-qualifier-list:
> @@ -6315,7 +6316,7 @@ static tree
>  c_parser_asm_statement (c_parser *parser)
>  {
>    tree quals, str, outputs, inputs, clobbers, labels, ret;
> -  bool simple, is_goto;
> +  bool simple, is_inline, is_goto;
>    location_t asm_loc = c_parser_peek_token (parser)->location;
>    int section, nsections;
>
> @@ -6323,6 +6324,7 @@ c_parser_asm_statement (c_parser *parser)
>    c_parser_consume_token (parser);
>
>    quals = NULL_TREE;
> +  is_inline = false;
>    is_goto = false;
>    for (bool done = false; !done; )
>      switch (c_parser_peek_token (parser)->keyword)
> @@ -6340,6 +6342,10 @@ c_parser_asm_statement (c_parser *parser)
>                     c_parser_peek_token (parser)->value);
>         c_parser_consume_token (parser);
>         break;
> +      case RID_INLINE:
> +       is_inline = true;
> +       c_parser_consume_token (parser);
> +       break;
>        case RID_GOTO:
>         is_goto = true;
>         c_parser_consume_token (parser);
> @@ -6423,7 +6429,8 @@ c_parser_asm_statement (c_parser *parser)
>      c_parser_skip_to_end_of_block_or_statement (parser);
>
>    ret = build_asm_stmt (quals, build_asm_expr (asm_loc, str, outputs, inputs,
> -                                              clobbers, labels, simple));
> +                                              clobbers, labels, simple,
> +                                              is_inline));
>
>   error:
>    parser->lex_untranslated_string = false;
> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
> index be63fee..2537d3e 100644
> --- a/gcc/c/c-tree.h
> +++ b/gcc/c/c-tree.h
> @@ -677,7 +677,8 @@ extern tree build_compound_literal (location_t, tree, tree, bool,
>  extern void check_compound_literal_type (location_t, struct c_type_name *);
>  extern tree c_start_case (location_t, location_t, tree, bool);
>  extern void c_finish_case (tree, tree);
> -extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool);
> +extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool,
> +                           bool);
>  extern tree build_asm_stmt (tree, tree);
>  extern int c_types_compatible_p (tree, tree);
>  extern tree c_begin_compound_stmt (bool);
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 9d09b8d..e013100 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -10064,7 +10064,7 @@ build_asm_stmt (tree cv_qualifier, tree args)
>     are subtly different.  We use a ASM_EXPR node to represent this.  */
>  tree
>  build_asm_expr (location_t loc, tree string, tree outputs, tree inputs,
> -               tree clobbers, tree labels, bool simple)
> +               tree clobbers, tree labels, bool simple, bool is_inline)
>  {
>    tree tail;
>    tree args;
> @@ -10182,6 +10182,7 @@ build_asm_expr (location_t loc, tree string, tree outputs, tree inputs,
>       as volatile.  */
>    ASM_INPUT_P (args) = simple;
>    ASM_VOLATILE_P (args) = (noutputs == 0);
> +  ASM_INLINE_P (args) = is_inline;
>
>    return args;
>  }
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 26ded3a..0bd5858 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6935,7 +6935,7 @@ extern tree begin_compound_stmt                   (unsigned int);
>
>  extern void finish_compound_stmt               (tree);
>  extern tree finish_asm_stmt                    (int, tree, tree, tree, tree,
> -                                                tree);
> +                                                tree, bool);
>  extern tree finish_label_stmt                  (tree);
>  extern void finish_label_decl                  (tree);
>  extern cp_expr finish_parenthesized_expr       (cp_expr);
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index d44fd4d..d5174f7 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -19198,6 +19198,7 @@ cp_parser_using_directive (cp_parser* parser)
>
>    asm-qualifier:
>      volatile
> +    inline
>      goto
>
>    asm-qualifier-list:
> @@ -19238,6 +19239,7 @@ cp_parser_asm_definition (cp_parser* parser)
>    bool extended_p = false;
>    bool invalid_inputs_p = false;
>    bool invalid_outputs_p = false;
> +  bool inline_p = false;
>    bool goto_p = false;
>    required_token missing = RT_NONE;
>
> @@ -19267,6 +19269,17 @@ cp_parser_asm_definition (cp_parser* parser)
>           else
>             done = true;
>           break;
> +       case RID_INLINE:
> +         if (!inline_p && parser->in_function_body)
> +           {
> +             /* Remember that we saw the `inline' keyword.  */
> +             inline_p = true;
> +             /* Consume the token.  */
> +             cp_lexer_consume_token (parser->lexer);
> +           }
> +         else
> +           done = true;
> +         break;
>         case RID_GOTO:
>           if (!goto_p && parser->in_function_body)
>             {
> @@ -19408,7 +19421,7 @@ cp_parser_asm_definition (cp_parser* parser)
>        if (parser->in_function_body)
>         {
>           asm_stmt = finish_asm_stmt (volatile_p, string, outputs,
> -                                     inputs, clobbers, labels);
> +                                     inputs, clobbers, labels, inline_p);
>           /* If the extended syntax was not used, mark the ASM_EXPR.  */
>           if (!extended_p)
>             {
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index f290cb3..4cd501b 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -17008,7 +17008,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
>         tree labels = tsubst_copy_asm_operands (ASM_LABELS (t), args,
>                                                 complain, in_decl);
>         tmp = finish_asm_stmt (ASM_VOLATILE_P (t), string, outputs, inputs,
> -                              clobbers, labels);
> +                              clobbers, labels, ASM_INLINE_P (t));
>         tree asm_expr = tmp;
>         if (TREE_CODE (asm_expr) == CLEANUP_POINT_EXPR)
>           asm_expr = TREE_OPERAND (asm_expr, 0);
> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index c7f53d1..fa792bd 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -1485,7 +1485,7 @@ finish_compound_stmt (tree stmt)
>
>  tree
>  finish_asm_stmt (int volatile_p, tree string, tree output_operands,
> -                tree input_operands, tree clobbers, tree labels)
> +                tree input_operands, tree clobbers, tree labels, bool inline_p)
>  {
>    tree r;
>    tree t;
> @@ -1639,6 +1639,7 @@ finish_asm_stmt (int volatile_p, tree string, tree output_operands,
>                   output_operands, input_operands,
>                   clobbers, labels);
>    ASM_VOLATILE_P (r) = volatile_p || noutputs == 0;
> +  ASM_INLINE_P (r) = inline_p;
>    r = maybe_cleanup_point_expr_void (r);
>    return add_stmt (r);
>  }
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 7aeb4fd..9e042e3 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -8220,6 +8220,9 @@ The extended form is preferred for mixing C and assembly language
>  within a function, but to include assembly language at
>  top level you must use basic @code{asm}.
>
> +You can use @code{asm inline} instead of @code{asm} to have the assembler
> +code counted as mimimum size for inlining purposes; @pxref{Size of an asm}.
> +
>  You can also use the @code{asm} keyword to override the assembler name
>  for a C symbol, or to place a C variable in a specific register.
>
> @@ -9853,7 +9856,7 @@ does this by counting the number of instructions in the pattern of the
>  @code{asm} and multiplying that by the length of the longest
>  instruction supported by that processor.  (When working out the number
>  of instructions, it assumes that any occurrence of a newline or of
> -whatever statement separator character is supported by the assembler --
> +whatever statement separator character is supported by the assembler ---
>  typically @samp{;} --- indicates the end of an instruction.)
>
>  Normally, GCC's estimate is adequate to ensure that correct
> @@ -9864,6 +9867,11 @@ space in the object file than is needed for a single instruction.
>  If this happens then the assembler may produce a diagnostic saying that
>  a label is unreachable.
>
> +@cindex @code{asm inline}
> +This size is also used for inlining decisions.  If you use @code{asm inline}
> +instead of just @code{asm}, then for inlining purposes the size of the asm
> +is taken as the minimum size.
> +
>  @node Alternate Keywords
>  @section Alternate Keywords
>  @cindex alternate keywords
> diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
> index 7dfec91..5b36ef2 100644
> --- a/gcc/gimple-pretty-print.c
> +++ b/gcc/gimple-pretty-print.c
> @@ -2019,6 +2019,8 @@ dump_gimple_asm (pretty_printer *buffer, gasm *gs, int spc, dump_flags_t flags)
>        pp_string (buffer, "__asm__");
>        if (gimple_asm_volatile_p (gs))
>         pp_string (buffer, " __volatile__");
> +      if (gimple_asm_inline_p (gs))
> +       pp_string (buffer, " __inline__");
>        if (gimple_asm_nlabels (gs))
>         pp_string (buffer, " goto");
>        pp_string (buffer, "(\"");
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index a5dda93..8a58e07 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -137,6 +137,7 @@ enum gimple_rhs_class
>  enum gf_mask {
>      GF_ASM_INPUT               = 1 << 0,
>      GF_ASM_VOLATILE            = 1 << 1,
> +    GF_ASM_INLINE              = 1 << 2,
>      GF_CALL_FROM_THUNK         = 1 << 0,
>      GF_CALL_RETURN_SLOT_OPT    = 1 << 1,
>      GF_CALL_TAILCALL           = 1 << 2,
> @@ -3911,7 +3912,7 @@ gimple_asm_volatile_p (const gasm *asm_stmt)
>  }
>
>
> -/* If VOLATLE_P is true, mark asm statement ASM_STMT as volatile.  */
> +/* If VOLATILE_P is true, mark asm statement ASM_STMT as volatile.  */
>
>  static inline void
>  gimple_asm_set_volatile (gasm *asm_stmt, bool volatile_p)
> @@ -3923,6 +3924,27 @@ gimple_asm_set_volatile (gasm *asm_stmt, bool volatile_p)
>  }
>
>
> +/* Return true ASM_STMT ASM_STMT is an asm statement marked inline.  */
> +
> +static inline bool
> +gimple_asm_inline_p (const gasm *asm_stmt)
> +{
> +  return (asm_stmt->subcode & GF_ASM_INLINE) != 0;
> +}
> +
> +
> +/* If INLINE_P is true, mark asm statement ASM_STMT as inline.  */
> +
> +static inline void
> +gimple_asm_set_inline (gasm *asm_stmt, bool inline_p)
> +{
> +  if (inline_p)
> +    asm_stmt->subcode |= GF_ASM_INLINE;
> +  else
> +    asm_stmt->subcode &= ~GF_ASM_INLINE;
> +}
> +
> +
>  /* If INPUT_P is true, mark asm ASM_STMT as an ASM_INPUT.  */
>
>  static inline void
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 509fc2f..10b80f2 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -6315,6 +6315,7 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
>
>        gimple_asm_set_volatile (stmt, ASM_VOLATILE_P (expr) || noutputs == 0);
>        gimple_asm_set_input (stmt, ASM_INPUT_P (expr));
> +      gimple_asm_set_inline (stmt, ASM_INLINE_P (expr));
>
>        gimplify_seq_add_stmt (pre_p, stmt);
>      }
> diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
> index ba39ea3..5361139 100644
> --- a/gcc/ipa-icf-gimple.c
> +++ b/gcc/ipa-icf-gimple.c
> @@ -993,6 +993,9 @@ func_checker::compare_gimple_asm (const gasm *g1, const gasm *g2)
>    if (gimple_asm_input_p (g1) != gimple_asm_input_p (g2))
>      return false;
>
> +  if (gimple_asm_inline_p (g1) != gimple_asm_inline_p (g2))
> +    return false;
> +
>    if (gimple_asm_ninputs (g1) != gimple_asm_ninputs (g2))
>      return false;
>
> diff --git a/gcc/testsuite/c-c++-common/torture/asm-inline.c b/gcc/testsuite/c-c++-common/torture/asm-inline.c
> new file mode 100644
> index 0000000..dea8965
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/torture/asm-inline.c
> @@ -0,0 +1,53 @@
> +/* { dg-do compile } */
> +/* -O0 does no inlining, and -O3 does it too aggressively for this test:  */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-O3" } { "" } }
> +/* The normal asm is not inlined:  */
> +/* { dg-final { scan-assembler-times "w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w" 2 } } */
> +/* But the asm inline is inlined:  */
> +/* { dg-final { scan-assembler-times "x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x" 8 } } */
> +
> +static void f(void)
> +{
> +  asm ("w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\n"
> +       "w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw");
> +}
> +
> +int f0(void) { f(); return 0; }
> +int f1(void) { f(); return 1; }
> +int f2(void) { f(); return 2; }
> +int f3(void) { f(); return 3; }
> +
> +static void fg(void)
> +{
> +  asm goto("w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\n"
> +          "w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw" :::: q);
> +  q: ;
> +}
> +
> +int fg0(void) { fg(); return 0; }
> +int fg1(void) { fg(); return 1; }
> +int fg2(void) { fg(); return 2; }
> +int fg3(void) { fg(); return 3; }
> +
> +static void g(void)
> +{
> +  asm inline("x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\n"
> +            "x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx");
> +}
> +
> +int g0(void) { g(); return 0; }
> +int g1(void) { g(); return 1; }
> +int g2(void) { g(); return 2; }
> +int g3(void) { g(); return 3; }
> +
> +static void gg(void)
> +{
> +  asm inline goto("x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\n"
> +                 "x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx" :::: q);
> +  q: ;
> +}
> +
> +int gg0(void) { gg(); return 0; }
> +int gg1(void) { gg(); return 1; }
> +int gg2(void) { gg(); return 2; }
> +int gg3(void) { gg(); return 3; }
> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> index 7df5c40..a99ca5d 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -1151,6 +1151,9 @@ struct GTY(()) tree_base {
>         OMP_CLAUSE_LINEAR_VARIABLE_STRIDE in
>            OMP_CLAUSE_LINEAR
>
> +       ASM_INLINE_P in
> +          ASM_EXPR
> +
>     side_effects_flag:
>
>         TREE_SIDE_EFFECTS in
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index 297fcd7..274a400 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -4108,6 +4108,9 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
>            with very long asm statements.  */
>         if (count > 1000)
>           count = 1000;
> +       /* If this asm is asm inline, count anything as minimum size.  */
> +       if (gimple_asm_inline_p (as_a <gasm *> (stmt)))
> +         count = !!count;
>         return MAX (1, count);
>        }
>
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 0ef96ba..f5252e5 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -1245,6 +1245,9 @@ extern tree maybe_wrap_with_location (tree, location_t);
>     ASM_OPERAND with no operands.  */
>  #define ASM_INPUT_P(NODE) (ASM_EXPR_CHECK (NODE)->base.static_flag)
>  #define ASM_VOLATILE_P(NODE) (ASM_EXPR_CHECK (NODE)->base.public_flag)
> +/* Nonzero if we want to consider this asm as minimum length and cost
> +   for inlining decisions.  */
> +#define ASM_INLINE_P(NODE) (ASM_EXPR_CHECK (NODE)->base.protected_flag)
>
>  /* COND_EXPR accessors.  */
>  #define COND_EXPR_COND(NODE)   (TREE_OPERAND (COND_EXPR_CHECK (NODE), 0))
> --
> 1.8.3.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-11-11 23:41       ` Martin Sebor
@ 2018-11-12  0:19         ` Segher Boessenkool
  0 siblings, 0 replies; 22+ messages in thread
From: Segher Boessenkool @ 2018-11-12  0:19 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches, jakub

On Sun, Nov 11, 2018 at 04:41:19PM -0700, Martin Sebor wrote:
> On 11/11/2018 03:00 PM, Segher Boessenkool wrote:
> >On Sun, Nov 11, 2018 at 02:33:34PM -0700, Martin Sebor wrote:
> >>On 10/30/2018 11:30 AM, Segher Boessenkool wrote:
> >>>The Linux kernel people want a feature that makes GCC pretend some
> >>>inline assembler code is tiny (while it would think it is huge), so
> >>>that such code will be inlined essentially always instead of
> >>>essentially never.
> >>>
> >>>This patch lets you say "asm inline" instead of just "asm", with the
> >>>result that that inline assembler is always counted as minimum cost
> >>>for inlining.  It implements this for C and C++.
> >>
> >>Rather than overloading the inline keyword I think it would be
> >>more in keeping both with the design of existing features to
> >>control inlining in GCC and with the way the languages are
> >>evolving to allow the always_inline (and perhaps also noinline)
> >>attribute to apply to asm statements.
> >
> >We already "overloaded" the volatile and goto keywords here (it's not
> >overloading, the contexts are completely disjoint).
> >
> >always_inline is a function attribute, and you want to make it a statement
> >attribute (which we do not currently support except for empty statements!)
> >as well.
> 
> Yes, I suggest using a statement attribute for this.  I believe
> it would be more appropriate that overloading the meaning of
> an existing keyword in this context.

It is *not* overloading the meaning in this context.  There *was* no
meaning in this context.

Also, all of the other keywords allowed here (const, volatile, restrict,
goto) have different meanings in other contexts already.

> C++ is entertaining proposals to introduce a few statement
> attributes (e.g., likely/unlikely and even unreachable) so
> as I said, adding one to GCC would be in like with
> the direction the languages are moving (C has adopted the C++
> 11 attributes for C2X and it likely will continue to do so going
> forward).

As I've said many times now, I think attributes are a bad match for this.
Also, I am not going to implement statement attributes just for this.

> Supporting an attribute in this context also has a lower cost
> for third party tools that parse code (and are already prepared
> to deal with attributes).  Adding a new keyword here has a good
> chance of breaking some of those tools.

I don't see why I would care.  Sorry.  Extended asm is a GCC extension,
so we make the rules.

The first patch makes any order and combination of qualifiers (and goto
and inline) valid for asm, where before only zero or one qualifiers and
zero or one goto were allowed (in that order).  This gives exactly the
same problem for 3rd party tools, but it is an obvious improvement (and
even a bugfix).

> >>The inline keyword is commonly understood to be just a hint to
> >>the compiler.
> >
> >That is exactly what it is here.  It hints the compiler that this asm
> >is cheap, whatever it may look like.
> 
> The way you described the purpose of asm inline: "such code will
> be inlined essentially always instead of essentially never"
> sounded to me like it's close to the effect of attribute
> always_inline.  But after reading the patch it looks like the asm
> just overrides the number of instructions GCC estimates
> the statement expands into.

Only for the inlining decisions.  It still uses the conservative
estimate for things like the length attribute (the machine description
thing), and other things required for correctness.

> If I read the code right then
> a different attribute altogether would seem more appropriate
> (e.g., perhaps even something like insn_count (N); that would
> make it possible to provide an exact instruction count and also
> prevent the inlining of the enclosing function by specifying
> a very large count).

No, the user does not know what units is counted in here.  Would there
be any real benefit anyway?  I don't see it.

> In any event, overloading the inline
> keyword for this purpose doesn't seem like a good approach
> to me.

It is not overloading anything.


Segher

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-11-11 22:01     ` Segher Boessenkool
@ 2018-11-11 23:41       ` Martin Sebor
  2018-11-12  0:19         ` Segher Boessenkool
  0 siblings, 1 reply; 22+ messages in thread
From: Martin Sebor @ 2018-11-11 23:41 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches, jakub

On 11/11/2018 03:00 PM, Segher Boessenkool wrote:
> On Sun, Nov 11, 2018 at 02:33:34PM -0700, Martin Sebor wrote:
>> On 10/30/2018 11:30 AM, Segher Boessenkool wrote:
>>> The Linux kernel people want a feature that makes GCC pretend some
>>> inline assembler code is tiny (while it would think it is huge), so
>>> that such code will be inlined essentially always instead of
>>> essentially never.
>>>
>>> This patch lets you say "asm inline" instead of just "asm", with the
>>> result that that inline assembler is always counted as minimum cost
>>> for inlining.  It implements this for C and C++.
>>
>> Rather than overloading the inline keyword I think it would be
>> more in keeping both with the design of existing features to
>> control inlining in GCC and with the way the languages are
>> evolving to allow the always_inline (and perhaps also noinline)
>> attribute to apply to asm statements.
>
> We already "overloaded" the volatile and goto keywords here (it's not
> overloading, the contexts are completely disjoint).
>
> always_inline is a function attribute, and you want to make it a statement
> attribute (which we do not currently support except for empty statements!)
> as well.

Yes, I suggest using a statement attribute for this.  I believe
it would be more appropriate that overloading the meaning of
an existing keyword in this context.

C++ is entertaining proposals to introduce a few statement
attributes (e.g., likely/unlikely and even unreachable) so
as I said, adding one to GCC would be in like with
the direction the languages are moving (C has adopted the C++
11 attributes for C2X and it likely will continue to do so going
forward).

Supporting an attribute in this context also has a lower cost
for third party tools that parse code (and are already prepared
to deal with attributes).  Adding a new keyword here has a good
chance of breaking some of those tools.

>> The inline keyword is commonly understood to be just a hint to
>> the compiler.
>
> That is exactly what it is here.  It hints the compiler that this asm
> is cheap, whatever it may look like.

The way you described the purpose of asm inline: "such code will
be inlined essentially always instead of essentially never"
sounded to me like it's close to the effect of attribute
always_inline.  But after reading the patch it looks like the asm
just overrides the number of instructions GCC estimates
the statement expands into.  If I read the code right then
a different attribute altogether would seem more appropriate
(e.g., perhaps even something like insn_count (N); that would
make it possible to provide an exact instruction count and also
prevent the inlining of the enclosing function by specifying
a very large count).  In any event, overloading the inline
keyword for this purpose doesn't seem like a good approach
to me.

Martin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-11-11 21:33   ` Martin Sebor
@ 2018-11-11 22:01     ` Segher Boessenkool
  2018-11-11 23:41       ` Martin Sebor
  0 siblings, 1 reply; 22+ messages in thread
From: Segher Boessenkool @ 2018-11-11 22:01 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches, jakub

On Sun, Nov 11, 2018 at 02:33:34PM -0700, Martin Sebor wrote:
> On 10/30/2018 11:30 AM, Segher Boessenkool wrote:
> >The Linux kernel people want a feature that makes GCC pretend some
> >inline assembler code is tiny (while it would think it is huge), so
> >that such code will be inlined essentially always instead of
> >essentially never.
> >
> >This patch lets you say "asm inline" instead of just "asm", with the
> >result that that inline assembler is always counted as minimum cost
> >for inlining.  It implements this for C and C++.
> 
> Rather than overloading the inline keyword I think it would be
> more in keeping both with the design of existing features to
> control inlining in GCC and with the way the languages are
> evolving to allow the always_inline (and perhaps also noinline)
> attribute to apply to asm statements.

We already "overloaded" the volatile and goto keywords here (it's not
overloading, the contexts are completely disjoint).

always_inline is a function attribute, and you want to make it a statement
attribute (which we do not currently support except for empty statements!)
as well.

> The inline keyword is commonly understood to be just a hint to
> the compiler.

That is exactly what it is here.  It hints the compiler that this asm
is cheap, whatever it may look like.

> The attributes, on the other hand, are recognized
> as binding requests to inline (if possible) or avoid inlining,
> respectively.

And that is not what this does (that would be hard to do, in fact).

> >+You can use @code{asm inline} instead of @code{asm} to have the assembler
> >+code counted as mimimum size for inlining purposes; @pxref{Size of an 
> >asm}.


Segher

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-10-30 18:41 ` [PATCH 2/2] asm inline Segher Boessenkool
  2018-10-30 18:58   ` Marek Polacek
@ 2018-11-11 21:33   ` Martin Sebor
  2018-11-11 22:01     ` Segher Boessenkool
  2018-11-30 13:14   ` Richard Biener
  2 siblings, 1 reply; 22+ messages in thread
From: Martin Sebor @ 2018-11-11 21:33 UTC (permalink / raw)
  To: Segher Boessenkool, gcc-patches; +Cc: jakub

On 10/30/2018 11:30 AM, Segher Boessenkool wrote:
> The Linux kernel people want a feature that makes GCC pretend some
> inline assembler code is tiny (while it would think it is huge), so
> that such code will be inlined essentially always instead of
> essentially never.
>
> This patch lets you say "asm inline" instead of just "asm", with the
> result that that inline assembler is always counted as minimum cost
> for inlining.  It implements this for C and C++.

Rather than overloading the inline keyword I think it would be
more in keeping both with the design of existing features to
control inlining in GCC and with the way the languages are
evolving to allow the always_inline (and perhaps also noinline)
attribute to apply to asm statements.

The inline keyword is commonly understood to be just a hint to
the compiler.  The attributes, on the other hand, are recognized
as binding requests to inline (if possible) or avoid inlining,
respectively.

Martin

>
>
> 2018-10-30  Segher Boessenkool  <segher@kernel.crashing.org>
>
>         * doc/extend.texi (Using Assembly Language with C): Document asm inline.
>         (Size of an asm): Fix typo.  Document asm inline.
>         * gimple-pretty-print.c (dump_gimple_asm): Handle asm inline.
>         * gimple.h (enum gf_mask): Add GF_ASM_INLINE.
>         (gimple_asm_set_volatile): Fix typo.
>         * gimple_asm_inline_p: New.
>         * gimple_asm_set_inline: New.
>         * gimplify.c (gimplify_asm_expr): Propagate the asm inline flag from
>         tree to gimple.
>         * ipa-icf-gimple.c (func_checker::compare_gimple_asm): Compare the
>         gimple_asm_inline_p flag, too.
>         * tree-core.h (tree_base): Document that protected_flag is ASM_INLINE_P
>         in an ASM_EXPR.
>         * tree-inline.c (estimate_num_insns): If gimple_asm_inline_p return
>         a minimum size for an asm.
>         * tree.h (ASM_INLINE_P): New.
>
> gcc/c/
>         * c-parser.c (c_parser_asm_statement): Detect the inline keyword
>         after asm.  Pass a flag for it to build_asm_expr.
>         * c-tree.h (build_asm_expr): Update declaration.
>         * c-typeck.c (build_asm_stmt): Add is_inline parameter.  Use it to
>         set ASM_INLINE_P.
>
> gcc/cp/
>         * cp-tree.h (finish_asm_stmt): Update declaration.
>         * parser.c (cp_parser_asm_definition): Detect the inline keyword
>         after asm.  Pass a flag for it to finish_asm_stmt.
>         * pt.c (tsubst_expr): Pass the ASM_INLINE_P flag to finish_asm_stmt.
>         * semantics.c (finish_asm_stmt): Add inline_p parameter.  Use it to
>         set ASM_INLINE_P.
>
> gcc/testsuite/
>         * c-c++-common/torture/asm-inline.c: New testcase.
>
> ---
>  gcc/c/c-parser.c                                | 15 +++++--
>  gcc/c/c-tree.h                                  |  3 +-
>  gcc/c/c-typeck.c                                |  3 +-
>  gcc/cp/cp-tree.h                                |  2 +-
>  gcc/cp/parser.c                                 | 15 ++++++-
>  gcc/cp/pt.c                                     |  2 +-
>  gcc/cp/semantics.c                              |  3 +-
>  gcc/doc/extend.texi                             | 10 ++++-
>  gcc/gimple-pretty-print.c                       |  2 +
>  gcc/gimple.h                                    | 24 ++++++++++-
>  gcc/gimplify.c                                  |  1 +
>  gcc/ipa-icf-gimple.c                            |  3 ++
>  gcc/testsuite/c-c++-common/torture/asm-inline.c | 53 +++++++++++++++++++++++++
>  gcc/tree-core.h                                 |  3 ++
>  gcc/tree-inline.c                               |  3 ++
>  gcc/tree.h                                      |  3 ++
>  16 files changed, 133 insertions(+), 12 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/torture/asm-inline.c
>
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index ce9921e..b28b712 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -6283,11 +6283,12 @@ c_parser_for_statement (c_parser *parser, bool ivdep, unsigned short unroll,
>  }
>
>  /* Parse an asm statement, a GNU extension.  This is a full-blown asm
> -   statement with inputs, outputs, clobbers, and volatile and goto tag
> -   allowed.
> +   statement with inputs, outputs, clobbers, and volatile, inline, and goto
> +   tags allowed.
>
>     asm-qualifier:
>       type-qualifier
> +     inline
>       goto
>
>     asm-qualifier-list:
> @@ -6315,7 +6316,7 @@ static tree
>  c_parser_asm_statement (c_parser *parser)
>  {
>    tree quals, str, outputs, inputs, clobbers, labels, ret;
> -  bool simple, is_goto;
> +  bool simple, is_inline, is_goto;
>    location_t asm_loc = c_parser_peek_token (parser)->location;
>    int section, nsections;
>
> @@ -6323,6 +6324,7 @@ c_parser_asm_statement (c_parser *parser)
>    c_parser_consume_token (parser);
>
>    quals = NULL_TREE;
> +  is_inline = false;
>    is_goto = false;
>    for (bool done = false; !done; )
>      switch (c_parser_peek_token (parser)->keyword)
> @@ -6340,6 +6342,10 @@ c_parser_asm_statement (c_parser *parser)
>  		    c_parser_peek_token (parser)->value);
>  	c_parser_consume_token (parser);
>  	break;
> +      case RID_INLINE:
> +	is_inline = true;
> +	c_parser_consume_token (parser);
> +	break;
>        case RID_GOTO:
>  	is_goto = true;
>  	c_parser_consume_token (parser);
> @@ -6423,7 +6429,8 @@ c_parser_asm_statement (c_parser *parser)
>      c_parser_skip_to_end_of_block_or_statement (parser);
>
>    ret = build_asm_stmt (quals, build_asm_expr (asm_loc, str, outputs, inputs,
> -					       clobbers, labels, simple));
> +					       clobbers, labels, simple,
> +					       is_inline));
>
>   error:
>    parser->lex_untranslated_string = false;
> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
> index be63fee..2537d3e 100644
> --- a/gcc/c/c-tree.h
> +++ b/gcc/c/c-tree.h
> @@ -677,7 +677,8 @@ extern tree build_compound_literal (location_t, tree, tree, bool,
>  extern void check_compound_literal_type (location_t, struct c_type_name *);
>  extern tree c_start_case (location_t, location_t, tree, bool);
>  extern void c_finish_case (tree, tree);
> -extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool);
> +extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool,
> +			    bool);
>  extern tree build_asm_stmt (tree, tree);
>  extern int c_types_compatible_p (tree, tree);
>  extern tree c_begin_compound_stmt (bool);
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 9d09b8d..e013100 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -10064,7 +10064,7 @@ build_asm_stmt (tree cv_qualifier, tree args)
>     are subtly different.  We use a ASM_EXPR node to represent this.  */
>  tree
>  build_asm_expr (location_t loc, tree string, tree outputs, tree inputs,
> -		tree clobbers, tree labels, bool simple)
> +		tree clobbers, tree labels, bool simple, bool is_inline)
>  {
>    tree tail;
>    tree args;
> @@ -10182,6 +10182,7 @@ build_asm_expr (location_t loc, tree string, tree outputs, tree inputs,
>       as volatile.  */
>    ASM_INPUT_P (args) = simple;
>    ASM_VOLATILE_P (args) = (noutputs == 0);
> +  ASM_INLINE_P (args) = is_inline;
>
>    return args;
>  }
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 26ded3a..0bd5858 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6935,7 +6935,7 @@ extern tree begin_compound_stmt			(unsigned int);
>
>  extern void finish_compound_stmt		(tree);
>  extern tree finish_asm_stmt			(int, tree, tree, tree, tree,
> -						 tree);
> +						 tree, bool);
>  extern tree finish_label_stmt			(tree);
>  extern void finish_label_decl			(tree);
>  extern cp_expr finish_parenthesized_expr	(cp_expr);
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index d44fd4d..d5174f7 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -19198,6 +19198,7 @@ cp_parser_using_directive (cp_parser* parser)
>
>    asm-qualifier:
>      volatile
> +    inline
>      goto
>
>    asm-qualifier-list:
> @@ -19238,6 +19239,7 @@ cp_parser_asm_definition (cp_parser* parser)
>    bool extended_p = false;
>    bool invalid_inputs_p = false;
>    bool invalid_outputs_p = false;
> +  bool inline_p = false;
>    bool goto_p = false;
>    required_token missing = RT_NONE;
>
> @@ -19267,6 +19269,17 @@ cp_parser_asm_definition (cp_parser* parser)
>  	  else
>  	    done = true;
>  	  break;
> +	case RID_INLINE:
> +	  if (!inline_p && parser->in_function_body)
> +	    {
> +	      /* Remember that we saw the `inline' keyword.  */
> +	      inline_p = true;
> +	      /* Consume the token.  */
> +	      cp_lexer_consume_token (parser->lexer);
> +	    }
> +	  else
> +	    done = true;
> +	  break;
>  	case RID_GOTO:
>  	  if (!goto_p && parser->in_function_body)
>  	    {
> @@ -19408,7 +19421,7 @@ cp_parser_asm_definition (cp_parser* parser)
>        if (parser->in_function_body)
>  	{
>  	  asm_stmt = finish_asm_stmt (volatile_p, string, outputs,
> -				      inputs, clobbers, labels);
> +				      inputs, clobbers, labels, inline_p);
>  	  /* If the extended syntax was not used, mark the ASM_EXPR.  */
>  	  if (!extended_p)
>  	    {
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index f290cb3..4cd501b 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -17008,7 +17008,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
>  	tree labels = tsubst_copy_asm_operands (ASM_LABELS (t), args,
>  						complain, in_decl);
>  	tmp = finish_asm_stmt (ASM_VOLATILE_P (t), string, outputs, inputs,
> -			       clobbers, labels);
> +			       clobbers, labels, ASM_INLINE_P (t));
>  	tree asm_expr = tmp;
>  	if (TREE_CODE (asm_expr) == CLEANUP_POINT_EXPR)
>  	  asm_expr = TREE_OPERAND (asm_expr, 0);
> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index c7f53d1..fa792bd 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -1485,7 +1485,7 @@ finish_compound_stmt (tree stmt)
>
>  tree
>  finish_asm_stmt (int volatile_p, tree string, tree output_operands,
> -		 tree input_operands, tree clobbers, tree labels)
> +		 tree input_operands, tree clobbers, tree labels, bool inline_p)
>  {
>    tree r;
>    tree t;
> @@ -1639,6 +1639,7 @@ finish_asm_stmt (int volatile_p, tree string, tree output_operands,
>  		  output_operands, input_operands,
>  		  clobbers, labels);
>    ASM_VOLATILE_P (r) = volatile_p || noutputs == 0;
> +  ASM_INLINE_P (r) = inline_p;
>    r = maybe_cleanup_point_expr_void (r);
>    return add_stmt (r);
>  }
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 7aeb4fd..9e042e3 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -8220,6 +8220,9 @@ The extended form is preferred for mixing C and assembly language
>  within a function, but to include assembly language at
>  top level you must use basic @code{asm}.
>
> +You can use @code{asm inline} instead of @code{asm} to have the assembler
> +code counted as mimimum size for inlining purposes; @pxref{Size of an asm}.
> +
>  You can also use the @code{asm} keyword to override the assembler name
>  for a C symbol, or to place a C variable in a specific register.
>
> @@ -9853,7 +9856,7 @@ does this by counting the number of instructions in the pattern of the
>  @code{asm} and multiplying that by the length of the longest
>  instruction supported by that processor.  (When working out the number
>  of instructions, it assumes that any occurrence of a newline or of
> -whatever statement separator character is supported by the assembler --
> +whatever statement separator character is supported by the assembler ---
>  typically @samp{;} --- indicates the end of an instruction.)
>
>  Normally, GCC's estimate is adequate to ensure that correct
> @@ -9864,6 +9867,11 @@ space in the object file than is needed for a single instruction.
>  If this happens then the assembler may produce a diagnostic saying that
>  a label is unreachable.
>
> +@cindex @code{asm inline}
> +This size is also used for inlining decisions.  If you use @code{asm inline}
> +instead of just @code{asm}, then for inlining purposes the size of the asm
> +is taken as the minimum size.
> +
>  @node Alternate Keywords
>  @section Alternate Keywords
>  @cindex alternate keywords
> diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
> index 7dfec91..5b36ef2 100644
> --- a/gcc/gimple-pretty-print.c
> +++ b/gcc/gimple-pretty-print.c
> @@ -2019,6 +2019,8 @@ dump_gimple_asm (pretty_printer *buffer, gasm *gs, int spc, dump_flags_t flags)
>        pp_string (buffer, "__asm__");
>        if (gimple_asm_volatile_p (gs))
>  	pp_string (buffer, " __volatile__");
> +      if (gimple_asm_inline_p (gs))
> +	pp_string (buffer, " __inline__");
>        if (gimple_asm_nlabels (gs))
>  	pp_string (buffer, " goto");
>        pp_string (buffer, "(\"");
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index a5dda93..8a58e07 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -137,6 +137,7 @@ enum gimple_rhs_class
>  enum gf_mask {
>      GF_ASM_INPUT		= 1 << 0,
>      GF_ASM_VOLATILE		= 1 << 1,
> +    GF_ASM_INLINE		= 1 << 2,
>      GF_CALL_FROM_THUNK		= 1 << 0,
>      GF_CALL_RETURN_SLOT_OPT	= 1 << 1,
>      GF_CALL_TAILCALL		= 1 << 2,
> @@ -3911,7 +3912,7 @@ gimple_asm_volatile_p (const gasm *asm_stmt)
>  }
>
>
> -/* If VOLATLE_P is true, mark asm statement ASM_STMT as volatile.  */
> +/* If VOLATILE_P is true, mark asm statement ASM_STMT as volatile.  */
>
>  static inline void
>  gimple_asm_set_volatile (gasm *asm_stmt, bool volatile_p)
> @@ -3923,6 +3924,27 @@ gimple_asm_set_volatile (gasm *asm_stmt, bool volatile_p)
>  }
>
>
> +/* Return true ASM_STMT ASM_STMT is an asm statement marked inline.  */
> +
> +static inline bool
> +gimple_asm_inline_p (const gasm *asm_stmt)
> +{
> +  return (asm_stmt->subcode & GF_ASM_INLINE) != 0;
> +}
> +
> +
> +/* If INLINE_P is true, mark asm statement ASM_STMT as inline.  */
> +
> +static inline void
> +gimple_asm_set_inline (gasm *asm_stmt, bool inline_p)
> +{
> +  if (inline_p)
> +    asm_stmt->subcode |= GF_ASM_INLINE;
> +  else
> +    asm_stmt->subcode &= ~GF_ASM_INLINE;
> +}
> +
> +
>  /* If INPUT_P is true, mark asm ASM_STMT as an ASM_INPUT.  */
>
>  static inline void
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 509fc2f..10b80f2 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -6315,6 +6315,7 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
>
>        gimple_asm_set_volatile (stmt, ASM_VOLATILE_P (expr) || noutputs == 0);
>        gimple_asm_set_input (stmt, ASM_INPUT_P (expr));
> +      gimple_asm_set_inline (stmt, ASM_INLINE_P (expr));
>
>        gimplify_seq_add_stmt (pre_p, stmt);
>      }
> diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
> index ba39ea3..5361139 100644
> --- a/gcc/ipa-icf-gimple.c
> +++ b/gcc/ipa-icf-gimple.c
> @@ -993,6 +993,9 @@ func_checker::compare_gimple_asm (const gasm *g1, const gasm *g2)
>    if (gimple_asm_input_p (g1) != gimple_asm_input_p (g2))
>      return false;
>
> +  if (gimple_asm_inline_p (g1) != gimple_asm_inline_p (g2))
> +    return false;
> +
>    if (gimple_asm_ninputs (g1) != gimple_asm_ninputs (g2))
>      return false;
>
> diff --git a/gcc/testsuite/c-c++-common/torture/asm-inline.c b/gcc/testsuite/c-c++-common/torture/asm-inline.c
> new file mode 100644
> index 0000000..dea8965
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/torture/asm-inline.c
> @@ -0,0 +1,53 @@
> +/* { dg-do compile } */
> +/* -O0 does no inlining, and -O3 does it too aggressively for this test:  */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-O3" } { "" } }
> +/* The normal asm is not inlined:  */
> +/* { dg-final { scan-assembler-times "w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w" 2 } } */
> +/* But the asm inline is inlined:  */
> +/* { dg-final { scan-assembler-times "x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x" 8 } } */
> +
> +static void f(void)
> +{
> +  asm ("w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\n"
> +       "w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw");
> +}
> +
> +int f0(void) { f(); return 0; }
> +int f1(void) { f(); return 1; }
> +int f2(void) { f(); return 2; }
> +int f3(void) { f(); return 3; }
> +
> +static void fg(void)
> +{
> +  asm goto("w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\n"
> +	   "w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw" :::: q);
> +  q: ;
> +}
> +
> +int fg0(void) { fg(); return 0; }
> +int fg1(void) { fg(); return 1; }
> +int fg2(void) { fg(); return 2; }
> +int fg3(void) { fg(); return 3; }
> +
> +static void g(void)
> +{
> +  asm inline("x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\n"
> +	     "x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx");
> +}
> +
> +int g0(void) { g(); return 0; }
> +int g1(void) { g(); return 1; }
> +int g2(void) { g(); return 2; }
> +int g3(void) { g(); return 3; }
> +
> +static void gg(void)
> +{
> +  asm inline goto("x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\n"
> +		  "x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx" :::: q);
> +  q: ;
> +}
> +
> +int gg0(void) { gg(); return 0; }
> +int gg1(void) { gg(); return 1; }
> +int gg2(void) { gg(); return 2; }
> +int gg3(void) { gg(); return 3; }
> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> index 7df5c40..a99ca5d 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -1151,6 +1151,9 @@ struct GTY(()) tree_base {
>         OMP_CLAUSE_LINEAR_VARIABLE_STRIDE in
>  	   OMP_CLAUSE_LINEAR
>
> +       ASM_INLINE_P in
> +	   ASM_EXPR
> +
>     side_effects_flag:
>
>         TREE_SIDE_EFFECTS in
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index 297fcd7..274a400 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -4108,6 +4108,9 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
>  	   with very long asm statements.  */
>  	if (count > 1000)
>  	  count = 1000;
> +	/* If this asm is asm inline, count anything as minimum size.  */
> +	if (gimple_asm_inline_p (as_a <gasm *> (stmt)))
> +	  count = !!count;
>  	return MAX (1, count);
>        }
>
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 0ef96ba..f5252e5 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -1245,6 +1245,9 @@ extern tree maybe_wrap_with_location (tree, location_t);
>     ASM_OPERAND with no operands.  */
>  #define ASM_INPUT_P(NODE) (ASM_EXPR_CHECK (NODE)->base.static_flag)
>  #define ASM_VOLATILE_P(NODE) (ASM_EXPR_CHECK (NODE)->base.public_flag)
> +/* Nonzero if we want to consider this asm as minimum length and cost
> +   for inlining decisions.  */
> +#define ASM_INLINE_P(NODE) (ASM_EXPR_CHECK (NODE)->base.protected_flag)
>
>  /* COND_EXPR accessors.  */
>  #define COND_EXPR_COND(NODE)	(TREE_OPERAND (COND_EXPR_CHECK (NODE), 0))
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] asm inline
  2018-10-30 18:41 ` [PATCH 2/2] asm inline Segher Boessenkool
@ 2018-10-30 18:58   ` Marek Polacek
  2018-11-11 21:33   ` Martin Sebor
  2018-11-30 13:14   ` Richard Biener
  2 siblings, 0 replies; 22+ messages in thread
From: Marek Polacek @ 2018-10-30 18:58 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches, jakub

On Tue, Oct 30, 2018 at 05:30:34PM +0000, Segher Boessenkool wrote:
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -10064,7 +10064,7 @@ build_asm_stmt (tree cv_qualifier, tree args)
>     are subtly different.  We use a ASM_EXPR node to represent this.  */
>  tree
>  build_asm_expr (location_t loc, tree string, tree outputs, tree inputs,
> -		tree clobbers, tree labels, bool simple)
> +		tree clobbers, tree labels, bool simple, bool is_inline)

The new parameter should probably be described in the comment above.

Marek

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/2] asm inline
  2018-10-30 18:01 [PATCH 0/2] asm qualifiers (PR55681) and asm input Segher Boessenkool
@ 2018-10-30 18:41 ` Segher Boessenkool
  2018-10-30 18:58   ` Marek Polacek
                     ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Segher Boessenkool @ 2018-10-30 18:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: jakub, Segher Boessenkool

The Linux kernel people want a feature that makes GCC pretend some
inline assembler code is tiny (while it would think it is huge), so
that such code will be inlined essentially always instead of
essentially never.

This patch lets you say "asm inline" instead of just "asm", with the
result that that inline assembler is always counted as minimum cost
for inlining.  It implements this for C and C++.


2018-10-30  Segher Boessenkool  <segher@kernel.crashing.org>

        * doc/extend.texi (Using Assembly Language with C): Document asm inline.
        (Size of an asm): Fix typo.  Document asm inline.
        * gimple-pretty-print.c (dump_gimple_asm): Handle asm inline.
        * gimple.h (enum gf_mask): Add GF_ASM_INLINE.
        (gimple_asm_set_volatile): Fix typo.
        * gimple_asm_inline_p: New.
        * gimple_asm_set_inline: New.
        * gimplify.c (gimplify_asm_expr): Propagate the asm inline flag from
        tree to gimple.
        * ipa-icf-gimple.c (func_checker::compare_gimple_asm): Compare the
        gimple_asm_inline_p flag, too.
        * tree-core.h (tree_base): Document that protected_flag is ASM_INLINE_P
        in an ASM_EXPR.
        * tree-inline.c (estimate_num_insns): If gimple_asm_inline_p return
        a minimum size for an asm.
        * tree.h (ASM_INLINE_P): New.

gcc/c/
        * c-parser.c (c_parser_asm_statement): Detect the inline keyword
        after asm.  Pass a flag for it to build_asm_expr.
        * c-tree.h (build_asm_expr): Update declaration.
        * c-typeck.c (build_asm_stmt): Add is_inline parameter.  Use it to
        set ASM_INLINE_P.

gcc/cp/
        * cp-tree.h (finish_asm_stmt): Update declaration.
        * parser.c (cp_parser_asm_definition): Detect the inline keyword
        after asm.  Pass a flag for it to finish_asm_stmt.
        * pt.c (tsubst_expr): Pass the ASM_INLINE_P flag to finish_asm_stmt.
        * semantics.c (finish_asm_stmt): Add inline_p parameter.  Use it to
        set ASM_INLINE_P.

gcc/testsuite/
        * c-c++-common/torture/asm-inline.c: New testcase.

---
 gcc/c/c-parser.c                                | 15 +++++--
 gcc/c/c-tree.h                                  |  3 +-
 gcc/c/c-typeck.c                                |  3 +-
 gcc/cp/cp-tree.h                                |  2 +-
 gcc/cp/parser.c                                 | 15 ++++++-
 gcc/cp/pt.c                                     |  2 +-
 gcc/cp/semantics.c                              |  3 +-
 gcc/doc/extend.texi                             | 10 ++++-
 gcc/gimple-pretty-print.c                       |  2 +
 gcc/gimple.h                                    | 24 ++++++++++-
 gcc/gimplify.c                                  |  1 +
 gcc/ipa-icf-gimple.c                            |  3 ++
 gcc/testsuite/c-c++-common/torture/asm-inline.c | 53 +++++++++++++++++++++++++
 gcc/tree-core.h                                 |  3 ++
 gcc/tree-inline.c                               |  3 ++
 gcc/tree.h                                      |  3 ++
 16 files changed, 133 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/torture/asm-inline.c

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index ce9921e..b28b712 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -6283,11 +6283,12 @@ c_parser_for_statement (c_parser *parser, bool ivdep, unsigned short unroll,
 }
 
 /* Parse an asm statement, a GNU extension.  This is a full-blown asm
-   statement with inputs, outputs, clobbers, and volatile and goto tag
-   allowed.
+   statement with inputs, outputs, clobbers, and volatile, inline, and goto
+   tags allowed.
 
    asm-qualifier:
      type-qualifier
+     inline
      goto
 
    asm-qualifier-list:
@@ -6315,7 +6316,7 @@ static tree
 c_parser_asm_statement (c_parser *parser)
 {
   tree quals, str, outputs, inputs, clobbers, labels, ret;
-  bool simple, is_goto;
+  bool simple, is_inline, is_goto;
   location_t asm_loc = c_parser_peek_token (parser)->location;
   int section, nsections;
 
@@ -6323,6 +6324,7 @@ c_parser_asm_statement (c_parser *parser)
   c_parser_consume_token (parser);
 
   quals = NULL_TREE;
+  is_inline = false;
   is_goto = false;
   for (bool done = false; !done; )
     switch (c_parser_peek_token (parser)->keyword)
@@ -6340,6 +6342,10 @@ c_parser_asm_statement (c_parser *parser)
 		    c_parser_peek_token (parser)->value);
 	c_parser_consume_token (parser);
 	break;
+      case RID_INLINE:
+	is_inline = true;
+	c_parser_consume_token (parser);
+	break;
       case RID_GOTO:
 	is_goto = true;
 	c_parser_consume_token (parser);
@@ -6423,7 +6429,8 @@ c_parser_asm_statement (c_parser *parser)
     c_parser_skip_to_end_of_block_or_statement (parser);
 
   ret = build_asm_stmt (quals, build_asm_expr (asm_loc, str, outputs, inputs,
-					       clobbers, labels, simple));
+					       clobbers, labels, simple,
+					       is_inline));
 
  error:
   parser->lex_untranslated_string = false;
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index be63fee..2537d3e 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -677,7 +677,8 @@ extern tree build_compound_literal (location_t, tree, tree, bool,
 extern void check_compound_literal_type (location_t, struct c_type_name *);
 extern tree c_start_case (location_t, location_t, tree, bool);
 extern void c_finish_case (tree, tree);
-extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool);
+extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool,
+			    bool);
 extern tree build_asm_stmt (tree, tree);
 extern int c_types_compatible_p (tree, tree);
 extern tree c_begin_compound_stmt (bool);
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 9d09b8d..e013100 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -10064,7 +10064,7 @@ build_asm_stmt (tree cv_qualifier, tree args)
    are subtly different.  We use a ASM_EXPR node to represent this.  */
 tree
 build_asm_expr (location_t loc, tree string, tree outputs, tree inputs,
-		tree clobbers, tree labels, bool simple)
+		tree clobbers, tree labels, bool simple, bool is_inline)
 {
   tree tail;
   tree args;
@@ -10182,6 +10182,7 @@ build_asm_expr (location_t loc, tree string, tree outputs, tree inputs,
      as volatile.  */
   ASM_INPUT_P (args) = simple;
   ASM_VOLATILE_P (args) = (noutputs == 0);
+  ASM_INLINE_P (args) = is_inline;
 
   return args;
 }
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 26ded3a..0bd5858 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6935,7 +6935,7 @@ extern tree begin_compound_stmt			(unsigned int);
 
 extern void finish_compound_stmt		(tree);
 extern tree finish_asm_stmt			(int, tree, tree, tree, tree,
-						 tree);
+						 tree, bool);
 extern tree finish_label_stmt			(tree);
 extern void finish_label_decl			(tree);
 extern cp_expr finish_parenthesized_expr	(cp_expr);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d44fd4d..d5174f7 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -19198,6 +19198,7 @@ cp_parser_using_directive (cp_parser* parser)
 
   asm-qualifier:
     volatile
+    inline
     goto
 
   asm-qualifier-list:
@@ -19238,6 +19239,7 @@ cp_parser_asm_definition (cp_parser* parser)
   bool extended_p = false;
   bool invalid_inputs_p = false;
   bool invalid_outputs_p = false;
+  bool inline_p = false;
   bool goto_p = false;
   required_token missing = RT_NONE;
 
@@ -19267,6 +19269,17 @@ cp_parser_asm_definition (cp_parser* parser)
 	  else
 	    done = true;
 	  break;
+	case RID_INLINE:
+	  if (!inline_p && parser->in_function_body)
+	    {
+	      /* Remember that we saw the `inline' keyword.  */
+	      inline_p = true;
+	      /* Consume the token.  */
+	      cp_lexer_consume_token (parser->lexer);
+	    }
+	  else
+	    done = true;
+	  break;
 	case RID_GOTO:
 	  if (!goto_p && parser->in_function_body)
 	    {
@@ -19408,7 +19421,7 @@ cp_parser_asm_definition (cp_parser* parser)
       if (parser->in_function_body)
 	{
 	  asm_stmt = finish_asm_stmt (volatile_p, string, outputs,
-				      inputs, clobbers, labels);
+				      inputs, clobbers, labels, inline_p);
 	  /* If the extended syntax was not used, mark the ASM_EXPR.  */
 	  if (!extended_p)
 	    {
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f290cb3..4cd501b 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -17008,7 +17008,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 	tree labels = tsubst_copy_asm_operands (ASM_LABELS (t), args,
 						complain, in_decl);
 	tmp = finish_asm_stmt (ASM_VOLATILE_P (t), string, outputs, inputs,
-			       clobbers, labels);
+			       clobbers, labels, ASM_INLINE_P (t));
 	tree asm_expr = tmp;
 	if (TREE_CODE (asm_expr) == CLEANUP_POINT_EXPR)
 	  asm_expr = TREE_OPERAND (asm_expr, 0);
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index c7f53d1..fa792bd 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -1485,7 +1485,7 @@ finish_compound_stmt (tree stmt)
 
 tree
 finish_asm_stmt (int volatile_p, tree string, tree output_operands,
-		 tree input_operands, tree clobbers, tree labels)
+		 tree input_operands, tree clobbers, tree labels, bool inline_p)
 {
   tree r;
   tree t;
@@ -1639,6 +1639,7 @@ finish_asm_stmt (int volatile_p, tree string, tree output_operands,
 		  output_operands, input_operands,
 		  clobbers, labels);
   ASM_VOLATILE_P (r) = volatile_p || noutputs == 0;
+  ASM_INLINE_P (r) = inline_p;
   r = maybe_cleanup_point_expr_void (r);
   return add_stmt (r);
 }
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 7aeb4fd..9e042e3 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -8220,6 +8220,9 @@ The extended form is preferred for mixing C and assembly language
 within a function, but to include assembly language at
 top level you must use basic @code{asm}.
 
+You can use @code{asm inline} instead of @code{asm} to have the assembler
+code counted as mimimum size for inlining purposes; @pxref{Size of an asm}.
+
 You can also use the @code{asm} keyword to override the assembler name
 for a C symbol, or to place a C variable in a specific register.
 
@@ -9853,7 +9856,7 @@ does this by counting the number of instructions in the pattern of the
 @code{asm} and multiplying that by the length of the longest
 instruction supported by that processor.  (When working out the number
 of instructions, it assumes that any occurrence of a newline or of
-whatever statement separator character is supported by the assembler --
+whatever statement separator character is supported by the assembler ---
 typically @samp{;} --- indicates the end of an instruction.)
 
 Normally, GCC's estimate is adequate to ensure that correct
@@ -9864,6 +9867,11 @@ space in the object file than is needed for a single instruction.
 If this happens then the assembler may produce a diagnostic saying that
 a label is unreachable.
 
+@cindex @code{asm inline}
+This size is also used for inlining decisions.  If you use @code{asm inline}
+instead of just @code{asm}, then for inlining purposes the size of the asm
+is taken as the minimum size.
+
 @node Alternate Keywords
 @section Alternate Keywords
 @cindex alternate keywords
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index 7dfec91..5b36ef2 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -2019,6 +2019,8 @@ dump_gimple_asm (pretty_printer *buffer, gasm *gs, int spc, dump_flags_t flags)
       pp_string (buffer, "__asm__");
       if (gimple_asm_volatile_p (gs))
 	pp_string (buffer, " __volatile__");
+      if (gimple_asm_inline_p (gs))
+	pp_string (buffer, " __inline__");
       if (gimple_asm_nlabels (gs))
 	pp_string (buffer, " goto");
       pp_string (buffer, "(\"");
diff --git a/gcc/gimple.h b/gcc/gimple.h
index a5dda93..8a58e07 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -137,6 +137,7 @@ enum gimple_rhs_class
 enum gf_mask {
     GF_ASM_INPUT		= 1 << 0,
     GF_ASM_VOLATILE		= 1 << 1,
+    GF_ASM_INLINE		= 1 << 2,
     GF_CALL_FROM_THUNK		= 1 << 0,
     GF_CALL_RETURN_SLOT_OPT	= 1 << 1,
     GF_CALL_TAILCALL		= 1 << 2,
@@ -3911,7 +3912,7 @@ gimple_asm_volatile_p (const gasm *asm_stmt)
 }
 
 
-/* If VOLATLE_P is true, mark asm statement ASM_STMT as volatile.  */
+/* If VOLATILE_P is true, mark asm statement ASM_STMT as volatile.  */
 
 static inline void
 gimple_asm_set_volatile (gasm *asm_stmt, bool volatile_p)
@@ -3923,6 +3924,27 @@ gimple_asm_set_volatile (gasm *asm_stmt, bool volatile_p)
 }
 
 
+/* Return true ASM_STMT ASM_STMT is an asm statement marked inline.  */
+
+static inline bool
+gimple_asm_inline_p (const gasm *asm_stmt)
+{
+  return (asm_stmt->subcode & GF_ASM_INLINE) != 0;
+}
+
+
+/* If INLINE_P is true, mark asm statement ASM_STMT as inline.  */
+
+static inline void
+gimple_asm_set_inline (gasm *asm_stmt, bool inline_p)
+{
+  if (inline_p)
+    asm_stmt->subcode |= GF_ASM_INLINE;
+  else
+    asm_stmt->subcode &= ~GF_ASM_INLINE;
+}
+
+
 /* If INPUT_P is true, mark asm ASM_STMT as an ASM_INPUT.  */
 
 static inline void
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 509fc2f..10b80f2 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -6315,6 +6315,7 @@ gimplify_asm_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 
       gimple_asm_set_volatile (stmt, ASM_VOLATILE_P (expr) || noutputs == 0);
       gimple_asm_set_input (stmt, ASM_INPUT_P (expr));
+      gimple_asm_set_inline (stmt, ASM_INLINE_P (expr));
 
       gimplify_seq_add_stmt (pre_p, stmt);
     }
diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index ba39ea3..5361139 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -993,6 +993,9 @@ func_checker::compare_gimple_asm (const gasm *g1, const gasm *g2)
   if (gimple_asm_input_p (g1) != gimple_asm_input_p (g2))
     return false;
 
+  if (gimple_asm_inline_p (g1) != gimple_asm_inline_p (g2))
+    return false;
+
   if (gimple_asm_ninputs (g1) != gimple_asm_ninputs (g2))
     return false;
 
diff --git a/gcc/testsuite/c-c++-common/torture/asm-inline.c b/gcc/testsuite/c-c++-common/torture/asm-inline.c
new file mode 100644
index 0000000..dea8965
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/torture/asm-inline.c
@@ -0,0 +1,53 @@
+/* { dg-do compile } */
+/* -O0 does no inlining, and -O3 does it too aggressively for this test:  */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O3" } { "" } }
+/* The normal asm is not inlined:  */
+/* { dg-final { scan-assembler-times "w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w.w" 2 } } */
+/* But the asm inline is inlined:  */
+/* { dg-final { scan-assembler-times "x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x.x" 8 } } */
+
+static void f(void)
+{
+  asm ("w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\n"
+       "w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw");
+}
+
+int f0(void) { f(); return 0; }
+int f1(void) { f(); return 1; }
+int f2(void) { f(); return 2; }
+int f3(void) { f(); return 3; }
+
+static void fg(void)
+{
+  asm goto("w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\n"
+	   "w\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw\nw" :::: q);
+  q: ;
+}
+
+int fg0(void) { fg(); return 0; }
+int fg1(void) { fg(); return 1; }
+int fg2(void) { fg(); return 2; }
+int fg3(void) { fg(); return 3; }
+
+static void g(void)
+{
+  asm inline("x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\n"
+	     "x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx");
+}
+
+int g0(void) { g(); return 0; }
+int g1(void) { g(); return 1; }
+int g2(void) { g(); return 2; }
+int g3(void) { g(); return 3; }
+
+static void gg(void)
+{
+  asm inline goto("x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\n"
+		  "x\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx\nx" :::: q);
+  q: ;
+}
+
+int gg0(void) { gg(); return 0; }
+int gg1(void) { gg(); return 1; }
+int gg2(void) { gg(); return 2; }
+int gg3(void) { gg(); return 3; }
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 7df5c40..a99ca5d 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1151,6 +1151,9 @@ struct GTY(()) tree_base {
        OMP_CLAUSE_LINEAR_VARIABLE_STRIDE in
 	   OMP_CLAUSE_LINEAR
 
+       ASM_INLINE_P in
+	   ASM_EXPR
+
    side_effects_flag:
 
        TREE_SIDE_EFFECTS in
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 297fcd7..274a400 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -4108,6 +4108,9 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
 	   with very long asm statements.  */
 	if (count > 1000)
 	  count = 1000;
+	/* If this asm is asm inline, count anything as minimum size.  */
+	if (gimple_asm_inline_p (as_a <gasm *> (stmt)))
+	  count = !!count;
 	return MAX (1, count);
       }
 
diff --git a/gcc/tree.h b/gcc/tree.h
index 0ef96ba..f5252e5 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1245,6 +1245,9 @@ extern tree maybe_wrap_with_location (tree, location_t);
    ASM_OPERAND with no operands.  */
 #define ASM_INPUT_P(NODE) (ASM_EXPR_CHECK (NODE)->base.static_flag)
 #define ASM_VOLATILE_P(NODE) (ASM_EXPR_CHECK (NODE)->base.public_flag)
+/* Nonzero if we want to consider this asm as minimum length and cost
+   for inlining decisions.  */
+#define ASM_INLINE_P(NODE) (ASM_EXPR_CHECK (NODE)->base.protected_flag)
 
 /* COND_EXPR accessors.  */
 #define COND_EXPR_COND(NODE)	(TREE_OPERAND (COND_EXPR_CHECK (NODE), 0))
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-12-06 18:54 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-02 16:38 [PATCH v2 0/2] asm qualifiers (PR55681) and asm inline Segher Boessenkool
2018-12-02 16:39 ` [PATCH 1/2] asm qualifiers (PR55681) Segher Boessenkool
2018-12-03 22:20   ` Joseph Myers
2018-12-05 21:47   ` Jason Merrill
2018-12-05 23:02     ` Segher Boessenkool
2018-12-02 16:40 ` [PATCH 2/2] asm inline Segher Boessenkool
2018-12-02 17:23   ` Marc Glisse
2018-12-02 17:45     ` Segher Boessenkool
2018-12-02 18:38       ` Marc Glisse
2018-12-04 15:31   ` Richard Sandiford
2018-12-06  3:03     ` Segher Boessenkool
2018-12-06 18:11 ` [PATCH v2 0/2] asm qualifiers (PR55681) and " Segher Boessenkool
2018-12-06 18:15   ` Jakub Jelinek
2018-12-06 18:19     ` Joseph Myers
2018-12-06 18:54     ` Segher Boessenkool
  -- strict thread matches above, loose matches on Subject: below --
2018-10-30 18:01 [PATCH 0/2] asm qualifiers (PR55681) and asm input Segher Boessenkool
2018-10-30 18:41 ` [PATCH 2/2] asm inline Segher Boessenkool
2018-10-30 18:58   ` Marek Polacek
2018-11-11 21:33   ` Martin Sebor
2018-11-11 22:01     ` Segher Boessenkool
2018-11-11 23:41       ` Martin Sebor
2018-11-12  0:19         ` Segher Boessenkool
2018-11-30 13:14   ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).