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