This showed up when debugging the testcase for PR119164.
RISC-V FRM mode-switching state machine has special handling for transitions
to and from a call_insn as FRM needs to saved/restored around calls (any
call is considered potentially FRM clobbering). Consider the following
canonical example where insns 2, 4, 6 come are due to user code, while
the rest of frm save/restore insns 1, 3, 5, 7 need to be generated for the
ABI semantics.
test_float_point_frm_static:
1: frrm a5 <--
2: fsrmi 2
3: fsrm a5 <--
4: call normalize_vl
5: frrm a5 <--
6: fsrmi 3
7: fsrm a5 <--
Current implementation of RISC-V TARGET_MODE_NEEDED has special handling
if the call_insn is last insn of BB, to ensure FRM save/reads are emitted
on all the edges. However it doesn't work as intended and is borderline
bogus for following reasons:
- It fails to detect call_insn as last of BB (PR119164 test) if the
next BB starts with a code label (say due to call being conditional).
Granted this is a deficiency of API next_nonnote_nondebug_insn_bb ()
which incorrectly returns next BB code_label as opposed to returning
NULL (and this behavior is kind of relied upon by much of gcc).
This causes missed/delayed state transition to DYN.
- If code is tightened to actually detect above such as:
- rtx_insn *insn = next_nonnote_nondebug_insn_bb (cur_insn);
- if (!insn)
+ if (BB_END (BLOCK_FOR_INSN (cur_insn)) == cur_insn)
edge insertion happens but ends up splitting the BB which generic
mode-sw doesn't expect and ends up hittng an ICE.
- TARGET_MODE_NEEDED hook typically don't modify the CFG.
- For abnormal edges, insert_insn_end_basic_block () is called, which
by design on encountering call_insn as last in BB, inserts new insn
BEFORE the call, not after.
So this is just all wrong and ripe for removal. Moreover there seems to
be no testsuite coverage for this code path at all. Results don't change
at all if this is removed.
The total number of FRM read/writes emitted (static count) across all
benchmarks of a SPEC2017 -Ofast -march=rv64gcv build decrease slightly
so its a net win even if minimal but the real gain is reduced complexity
and maintenance.
Before Patch
---------------- ---------------
frrm fsrmi fsrm frrm fsrmi frrm
perlbench_r 42 0 4 42 0 4
cpugcc_r 167 0 17 167 0 17
bwaves_r 16 0 1 16 0 1
mcf_r 11 0 0 11 0 0
cactusBSSN_r 79 0 27 76 0 27
namd_r 119 0 63 119 0 63
parest_r 218 0 114 168 0 114 <--
povray_r 123 1 17 123 1 17
lbm_r 6 0 0 6 0 0
omnetpp_r 17 0 1 17 0 1
wrf_r 2287 13 1956 2287 13 1956
cpuxalan_r 17 0 1 17 0 1
ldecod_r 11 0 0 11 0 0
x264_r 14 0 1 14 0 1
blender_r 724 12 182 724 12 182
cam4_r 324 13 169 324 13 169
deepsjeng_r 11 0 0 11 0 0
imagick_r 265 16 34 265 16 34
leela_r 12 0 0 12 0 0
nab_r 13 0 1 13 0 1
exchange2_r 16 0 1 16 0 1
fotonik3d_r 20 0 11 20 0 11
roms_r 33 0 23 33 0 23
xz_r 6 0 0 6 0 0
---------------- ---------------
4551 55 2623 4498 55 2623
gcc/ChangeLog:
* config/riscv/riscv.cc (riscv_frm_emit_after_bb_end): Delete.
(riscv_frm_mode_needed): Remove call riscv_frm_emit_after_bb_end.
Signed-off-by: Vineet Gupta <vine...@rivosinc.com>
---
gcc/config/riscv/riscv.cc | 46 ---------------------------------------
1 file changed, 46 deletions(-)
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 62ec95d3b885..a500b046cd9a 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -12162,45 +12162,6 @@ riscv_frm_adjust_mode_after_call (rtx_insn *cur_insn,
int mode)
return mode;
}
-/* 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_emit_after_bb_end (rtx_insn *cur_insn)
-{
- edge eg;
- bool abnormal_edge_p = false;
- edge_iterator eg_iterator;
- basic_block bb = BLOCK_FOR_INSN (cur_insn);
-
- FOR_EACH_EDGE (eg, eg_iterator, bb->succs)
- {
- if (eg->flags & EDGE_ABNORMAL)
- abnormal_edge_p = true;
- else
- {
- start_sequence ();
- emit_insn (gen_frrmsi (DYNAMIC_FRM_RTL (cfun)));
- rtx_insn *backup_insn = get_insns ();
- end_sequence ();
-
- insert_insn_on_edge (backup_insn, eg);
- }
- }
-
- if (abnormal_edge_p)
- {
- start_sequence ();
- emit_insn (gen_frrmsi (DYNAMIC_FRM_RTL (cfun)));
- rtx_insn *backup_insn = get_insns ();
- end_sequence ();
-
- insert_insn_end_basic_block (backup_insn, bb);
- }
-
- commit_edge_insertions ();
-}
-
/* Return mode that frm must be switched into
prior to the execution of insn. */
@@ -12215,14 +12176,7 @@ riscv_frm_mode_needed (rtx_insn *cur_insn, int code)
}
if (CALL_P (cur_insn))
- {
- rtx_insn *insn = next_nonnote_nondebug_insn_bb (cur_insn);
-
- if (!insn)
- riscv_frm_emit_after_bb_end (cur_insn);
-
return riscv_vector::FRM_DYN_CALL;
- }
int mode = code >= 0 ? get_attr_frm_mode (cur_insn) : riscv_vector::FRM_NONE;
--
2.43.0
