| Message ID | e59a75e62603e73c299550dc50b2f61a668f53ca.1540918534.git.segher@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
| Series | asm qualifiers (PR55681) and asm input | expand |
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
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)) >
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
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
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
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 >
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))
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