Re: [PATCH 1/2] Add new RTX instruction class FILLER_INSN

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi Andrea,

On Wed, Jul 22, 2020 at 12:02:33PM +0200, Andrea Corallo wrote:
> This first patch implements the addition of a new RTX instruction class
> FILLER_INSN, which has been white listed to allow placement of NOPs
> outside of a basic block.  This is to allow padding after unconditional
> branches.  This is favorable so that any performance gained from
> diluting branches is not paid straight back via excessive eating of
> nops.

> It was deemed that a new RTX class was less invasive than modifying
> behavior in regards to standard UNSPEC nops.

So I wonder if this cannot be done with some kind of NOTE, instead?

> +	      /* Allow nops after branches, via FILLER_INSN.  */
> +	      bool fail = true;
> +	      subrtx_iterator::array_type array;
> +	      FOR_EACH_SUBRTX (iter, array, x, ALL)
> +		{
> +		  const_rtx rtx = *iter;
> +		  if (GET_CODE (rtx) == FILLER_INSN)
> +		    {
> +		      fail = false;
> +		      break;
> +		    }
> +		}
> +	      if (fail)
> +		fatal_insn ("insn outside basic block", x);

This stops checking after finding a FILLER_INSN as first insn.  Is that
on purpose?

> +/* Make an insn of code FILLER_INSN to
> +   pad out the instruction stream.
> +   PATTERN should be from gen_filler_insn ().
> +   AFTER will typically be an unconditional
> +   branch at the end of a basic block.  */

Because it only allows one particular insn pattern, it should be pretty
easy to use a NOTE for this, or even a normal INSN where the ouside-of-BB
test can then just look at its pattern.


As you see, I really do not like to have another RTX class, without very
well defined semantics even.  Not without first being shown no
alternatives are acceptable, anyway :-)


Segher