Re: [PATCH] RISC-V: Handle no_insn in TARGET_SCHED_VARIABLE_ISSUE.

[Jeff Law <jeffreyalaw@gmail.com>]

On 5/29/23 05:01, Jin Ma wrote:
> Reference: https://github.com/gcc-mirror/gcc/commit/d0bc0cb66bcb0e6a5a5a31a9e900e8ccc98e34e5
> 
> RISC-V should also be implemented to handle no_insn patterns for pipelining.
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/riscv.cc (riscv_sched_variable_issue): New function.
> 	(TARGET_SCHED_VARIABLE_ISSUE): New macro.
> ---
>   gcc/config/riscv/riscv.cc | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 3954fc07a8b..559fa9cd7e0 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -6225,6 +6225,24 @@ riscv_issue_rate (void)
>     return tune_param->issue_rate;
>   }
>   
> +/* Implement TARGET_SCHED_VARIABLE_ISSUE.  */
> +
> +static int
> +riscv_sched_variable_issue (FILE *, int, rtx_insn *insn, int more)
> +{
> +  if (DEBUG_INSN_P (insn))
> +    return more;
> +
> +  rtx_code code = GET_CODE (PATTERN (insn));
> +  if (code == USE || code == CLOBBER)
> +    return more;
> +
> +  if (get_attr_type (insn) == TYPE_UNKNOWN)
> +    return more;
> +
> +  return more - 1;
> +}
The problem is that INSN is *much* more likely to be a real instruction 
that takes real resources, even if it is TYPE_UNKNOWN.
TYPE_UNKNOWN here is actually an indicator of what I would consider a 
bug in the backend, specifically that we have INSNs that do not provide 
a mapping for the schedulers to suitable types.

With that in mind I'd much rather get to the point where we can do 
something like this for TYPE_UNKNOWN:

type = get_attr_type (insn);
gcc_assert (type != TYPE_UNKNOWN);

That way if we ever encounter a TYPE_UNKNOWN during development, we can 
fix it in the md files in a sensible manner.  I don't know if we are 
close to being able to do that.  We fixed a ton of stuff in bitmanip.md, 
but I don't think there's been a thorough review of the port to find 
other instances of TYPE_UNKNOWN INSNs.


The other thing if this code probably wants to handle GHOST type 
instructions.  While GHOST is used for instructions which generate no 
code, it might seem they should return "more" as those INSNs take no 
resources.  But GHOST is actually used for things like the blockage insn 
which should end a cycle from an issue standpoint.  So the right 
handling of a GHOST is something like this:

if (type == TYPE_GHOST)
   return 0;

Jeff