Re: [PATCH] asm non-code template parts (alternative to asm inline)

[Alexander Monakov <amonakov at ispras dot ru>]

> 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.

Ping - as I think this approach addresses the root of the problem, I wouldn't
like it to be forgotten.

> > 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);
>