On Mon, 15 Oct 2018, Richard Biener wrote: > I think it's sound but also note that I think it is logically independent of > asm inline (). While it may work for the inlining issue for some kernel > examples to asm inline () is sth similar to always_inline for functions, > that is, even though an asm _does_ have non-negligible .text size > we do want to ignore that for the purpose of inlining (but not for the > purpose of branch size estimation).
My understanding is that kernel folks are not demanding "always_inline" semantics though; they were unhappy with inlining cost mis-estimation. FWIW they went ahead and worked around the problem on their end via assembler macros (and Borislav's ping has got no response so far). > Note in your docs you refer to "non-code" sections but it should > equally apply to .text sections that are not the section of the asm > context (.text.cold, for example). So better wording there would > be appreciated. I've tried to address this with the following incremental change. I think it was the only suggestion, so the rest of the patch is unchanged. @@ -9849,11 +9849,11 @@ a label is unreachable. Likewise, it is possible for GCC to significantly over-estimate the number of instructions in an @code{asm}, resulting in suboptimal decisions when the estimate is used during inlining and branch range optimization. -This can happen if the @code{asm} template has many lines that do not -correspond to instructions, for example when the @samp{.pushsection} -directive is used to emit auxiliary data in a non-code section. +For instance, this can happen if a @samp{.pushsection} directive is used to +temporarily switch to another section and emit auxiliary code or data there. For Extended @code{asm} statements, you can improve the estimate by -wrapping the non-code portion in @samp{%` ... %`} delimiters. +wrapping the parts where newlines and separators should not be counted in +@samp{%` ... %`} delimiters. * doc/extend.texi (Extended Asm): Document %` in template. (Size of an Asm): Document intended use of %`. * final.c (asm_insn_count): Adjust. (asm_str_count): Add argument to distinguish basic and extended asms. In extended asms, ignore separators inside of %` ... %`. (output_asm_insn): Handle %`. * rtl.h (asm_str_count): Adjust prototype. * tree-inline.c (estimate_num_insns): Adjust. * config/arm/arm.c (arm_rtx_costs_internal): Adjust. diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 34ecc4f8d14..dac4728375b 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -8622,6 +8622,11 @@ generates multiple assembler instructions. Outputs @samp{@{}, @samp{|}, and @samp{@}} characters (respectively) into the assembler code. When unescaped, these characters have special meaning to indicate multiple assembler dialects, as described below. + +@item %` +Signifies a boundary of a region where instruction separators are not +counted towards its size (@pxref{Size of an asm}). Must be followed by +a whitespace character. @end table @subsubheading Multiple assembler dialects in @code{asm} templates @@ -9830,7 +9835,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 @@ -9841,6 +9846,15 @@ 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. +Likewise, it is possible for GCC to significantly over-estimate the +number of instructions in an @code{asm}, resulting in suboptimal decisions +when the estimate is used during inlining and branch range optimization. +For instance, this can happen if a @samp{.pushsection} directive is used to +temporarily switch to another section and emit auxiliary code or data there. +For Extended @code{asm} statements, you can improve the estimate by +wrapping the parts where newlines and separators should not be counted in +@samp{%` ... %`} delimiters. + @node Alternate Keywords @section Alternate Keywords @cindex alternate keywords diff --git a/gcc/final.c b/gcc/final.c index 6e61f1e17a8..1953293d379 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -1408,29 +1408,40 @@ static int asm_insn_count (rtx body) { const char *templ; + bool basic = GET_CODE (body) == ASM_INPUT; - if (GET_CODE (body) == ASM_INPUT) + if (basic) templ = XSTR (body, 0); else templ = decode_asm_operands (body, NULL, NULL, NULL, NULL, NULL); - return asm_str_count (templ); + return asm_str_count (templ, basic); } /* Return the number of machine instructions likely to be generated for the - inline-asm template. */ + inline-asm template. BASIC indicates if it is used in a basic asm. */ int -asm_str_count (const char *templ) +asm_str_count (const char *templ, bool basic) { int count = 1; + bool in_backticks = false; if (!*templ) return 0; for (; *templ; templ++) - if (IS_ASM_LOGICAL_LINE_SEPARATOR (*templ, templ) - || *templ == '\n') - count++; + if (*templ == '%' && !basic) + { + templ++; + if (!*templ) + break; + /* Separators inside %` ... %` are not counted. */ + if (*templ == '`') + in_backticks = !in_backticks; + } + else if (IS_ASM_LOGICAL_LINE_SEPARATOR (*templ, templ) + || *templ == '\n') + count += !in_backticks; return count; } @@ -3825,6 +3836,7 @@ output_asm_insn (const char *templ, rtx *operands) int oporder[MAX_RECOG_OPERANDS]; char opoutput[MAX_RECOG_OPERANDS]; int ops = 0; + bool in_backticks = false; /* An insn may return a null string template in a case where no assembler code is needed. */ @@ -3891,6 +3903,17 @@ output_asm_insn (const char *templ, rtx *operands) p++; fprintf (asm_out_file, "%d", insn_counter); } + /* %` guards parts of the template that should not participate in + estimating its cost, e.g. where it switches to a non-text section. + It does not result in any output. */ + else if (*p == '`') + { + p++; + in_backticks = !in_backticks; + /* Leave room for future extensions. */ + if (*p && !ISSPACE (*p)) + output_operand_lossage ("%%` must be followed by whitespace"); + } /* % followed by a letter and some digits outputs an operand in a special way depending on the letter. Letters `acln' are implemented directly. @@ -3984,6 +4007,9 @@ output_asm_insn (const char *templ, rtx *operands) output_asm_name (); putc ('\n', asm_out_file); + + if (in_backticks) + output_operand_lossage ("missing closing %%`"); } /* Output a LABEL_REF, or a bare CODE_LABEL, as an assembler symbol. */ diff --git a/gcc/rtl.h b/gcc/rtl.h index 68d3ceab29f..d29229d2817 100644 --- a/gcc/rtl.h +++ b/gcc/rtl.h @@ -4288,7 +4288,7 @@ extern void simplify_using_condition (rtx, rtx *, bitmap); /* In final.c */ extern unsigned int compute_alignments (void); extern void update_alignments (vec<rtx> &); -extern int asm_str_count (const char *templ); +extern int asm_str_count (const char *, bool); struct rtl_hooks { diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 913425394e0..70004528339 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -4103,7 +4103,9 @@ estimate_num_insns (gimple *stmt, eni_weights *weights) case GIMPLE_ASM: { - int count = asm_str_count (gimple_asm_string (as_a <gasm *> (stmt))); + gasm *asm_stmt = as_a <gasm *> (stmt); + const char *templ = gimple_asm_string (asm_stmt); + int count = asm_str_count (templ, gimple_asm_input_p (asm_stmt)); /* 1000 means infinity. This avoids overflows later with very long asm statements. */ if (count > 1000) diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 8810df53aa3..63c35400dc9 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -11026,7 +11026,8 @@ arm_rtx_costs_internal (rtx x, enum rtx_code code, enum rtx_code outer_code, /* Just a guess. Guess number of instructions in the asm plus one insn per input. Always a minimum of COSTS_N_INSNS (1) though (see PR60663). */ - int asm_length = MAX (1, asm_str_count (ASM_OPERANDS_TEMPLATE (x))); + const char *templ = ASM_OPERANDS_TEMPLATE (x); + int asm_length = MAX (1, asm_str_count (templ, false)); int num_operands = ASM_OPERANDS_INPUT_LENGTH (x); *cost = COSTS_N_INSNS (asm_length + num_operands);