> On 5/29/23 06:46, Jeff Law wrote:
> > 
> > 
> > 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;
> So there wasn't ever any follow-up.  Given this was something Ventana 
> was also carrying locally (with very minor differences) I went ahead and 
> merged up the implementations and pushed the final result to the trunk.
> 
> 
> Attached is the patch that was actually committed.
> 
> Jeff

My fault, I'm very sorry for not replying to the patch follow-up, I just
forgot this :)

Reply via email to