RISC-V: frm/mode-switch: remove dubious frm edge insertion before call_insn

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
despite it not being a callee-saved reg; rather it's a "global" reg
which can be temporarily modified "locally" with a static RM.
Thus a call needs to see the prior global state, hence the restore (from a
prior backup) before the call. Corollarily any call can potentially clobber
the FRM, thus post-call it needs to be it needs to be re-read/saved.

The following example demostrate this:
 - insns 2, 4, 6 correspond to actual user code,
 - rest 1, 3, 5, 6 are frm save/restore insns generated by mode switch for the
   above described 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 <vineetg@rivosinc.com>

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index d032578f19a42993ce0974e47e80cedab205e189..a1bb51af2be4bcca89368e936304353747edc16d 100644 (file)
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -12318,43 +12318,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 = 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 = 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.  */
 
@@ -12369,14 +12332,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;