Thanks Jeff for comments, and sorry for late response.
The background comes from the CALL insn. For the RISC-V dynamic rounding mode
we need to
1. restore the frm BEFORE call, to avoid the static rounding mode pollute the
call.
2. Backup the frm AFTER call, to ensure the frm value after call is live.
Currently, we don’t take care of it elegantly but we would like to refine this
part by the optional EMIT_AFTER.
> I'm not aware of a case where we can have an insn with control flow that
> isn't the end of the block. So perhaps then that second conditional
> into an assertion inside the true arm?
Not very sure my understanding is correct, but there may be a call insn in the
middle of the bb,
And can be considered as control flow?
> Is this really correct for EDGE_ABNORMAL? If the abnormal edge is
> created by, say a nonlocal goto, exception handling, etc, then the insn
> you insert at the end of the block will never be executed.
Got it, let me have a try for this, as well as there is somewhere take care of
this already.
Pan
-----Original Message-----
From: Jeff Law <jeffreya...@gmail.com>
Sent: Monday, August 21, 2023 10:24 PM
To: Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org
Cc: juzhe.zh...@rivai.ai; Wang, Yanzhang <yanzhang.w...@intel.com>;
kito.ch...@gmail.com
Subject: Re: [PATCH v1] Mode-Switching: Add optional EMIT_AFTER hook
On 8/21/23 01:26, pan2...@intel.com wrote:
> From: Pan Li <pan2...@intel.com>
>
> We have EMIT hook in mode switching already, which will insert the
> insn before in most cases. However, in some arch like RISC-V, it
> requires the additional insn to be inserted after when meet a call.
>
> |
> | <- EMIT HOOK, insert the insn before.
> +-----------+
> | ptr->insn |
> +-----------+
> | <- EMIT_AFTER HOOK, insert the insn after.
> |
>
> Thus, this patch would like to add one optional EMIT_AFTER hook, which
> will try to insert the emitted insn after. The end-user can either
> implement this HOOK or leave it NULL as is.
>
> If the backend ignore this optinal hook, there is no impact to the
> original mode switching stuff. If the backend implement this optional
> hook, the mode switching will try to insert the insn after. Please note
> the EMIT_AFTER doen't have any impact to EMIT hook.
>
> Passed both the regression and bootstrap test in x86.
>
> Signed-off-by: Pan Li <pan2...@intel.com>
>
> gcc/ChangeLog:
>
> * doc/tm.texi: Add hook def and update the description.
> * doc/tm.texi.in: Ditto.
> * mode-switching.cc (optimize_mode_switching): Insert the
> emitted insn after ptr->insn.
> * target.def (insn): Define emit_after hook.
Not a full review. I think I need to know a bit more about why you need
these additional hooks.
Presumably you can't use the current ".emit" hook because it doesn't
give you access to the block or insn that you can then iterate on for
insertion on the outgoing edges?
> @@ -831,6 +833,49 @@ optimize_mode_switching (void)
> emit_insn_before (mode_set, ptr->insn_ptr);
> }
>
> + if (targetm.mode_switching.emit_after)
> + {
> + if (control_flow_insn_p (ptr->insn_ptr)
> + && ptr->insn_ptr == BB_END (bb))
I'm not aware of a case where we can have an insn with control flow that
isn't the end of the block. So perhaps then that second conditional
into an assertion inside the true arm?
> + {
> + edge eg;
> + edge_iterator eg_iterator;
> +
> + FOR_EACH_EDGE (eg, eg_iterator, bb->succs)
> + {
> + start_sequence ();
> + targetm.mode_switching.emit_after (entity_map[j],
> + ptr->mode, cur_mode, ptr->regs_live);
> + mode_set = get_insns ();
> + end_sequence ();
> +
> + if (mode_set != NULL_RTX)
> + {
> + if (eg->flags & EDGE_ABNORMAL)
> + insert_insn_end_basic_block (mode_set, bb);
> + else
> + insert_insn_on_edge (mode_set, eg);
Is this really correct for EDGE_ABNORMAL? If the abnormal edge is
created by, say a nonlocal goto, exception handling, etc, then the insn
you insert at the end of the block will never be executed.
This is a classic problem with these classes of algorithms and I suspect
there's code elsewhere to deal with these cases.
Jeff
