Re: [PATCH v6] RISC-V: Support CALL for RVV floating-point dynamic rounding

[Robin Dapp <rdapp.gcc@gmail.com>]

Hi Pan,

> +  for (insn = PREV_INSN (cur_insn); insn; insn = PREV_INSN (insn))
> +    {
> +      if (INSN_P (insn))
> +	{
> +	  if (CALL_P (insn))
> +	    mode = FRM_MODE_DYN;
> +	  break;
> +	}
> +
> +      if (insn == BB_HEAD (bb))
> +	break;
> +    }
> +
> +  return mode;
> +}

Does prev_nonnote_nondebug_insn_bb help here?  In general, we scan
back here to the last insn and "recover" if it was a call?  Why
can't we set the proper value already before exiting the function?

I guess the more general question is more towards call-clobbered or
not?  In this patch we assume the rounding mode is call clobbered
and restore it ourselves.  Has there been any kind of consensus
on this?  Intuitively I would have expected a function that requires
a non-standard rounding mode to set and restore it itself. 

> +
> +/* Insert the backup frm insn to the end of the bb if and only if the call
> +   is the last insn of this bb.  */
> +
> +static void
> +riscv_frm_reconcile_call_as_bb_end (rtx_insn *cur_insn)
> +{
> +  rtx_insn *insn;
> +  basic_block bb = BLOCK_FOR_INSN (cur_insn);
> +
> +  gcc_assert (CALL_P (cur_insn));
> +
> +  if (cur_insn != BB_END (bb))
> +    {
> +      for (insn = NEXT_INSN (cur_insn); insn; insn = NEXT_INSN (insn))
> +	{
> +	  if (INSN_P (insn)) /* If there is one insn after call, do nothing.  */
> +	    return;

What about nondebug insns?  Also maybe next_nonnote_nondebug_insn_bb
helps?  How about splitting the function in detection and emit?
I.e. bb_ends_in_call (or so) and the emit part.  That way it
could be more obvious in "needed" what needs to be done.

Are we handling sibcalls and tail calls properly here?

In general I'm still a bit wary that we are checking mode != prev_mode
but cannot really pinpoint why.  I would have hoped that the generic
code only calls us when this check is unnecessary and if not the
"needed" hook should be adjusted.

I also find it a bit odd to emit instructions when checking
if another mode is needed.  If what's required cannot be accomplished
with the current common code, shouldn't we rather try to amend that?

Regards
 Robin