[committed,PR99781] Update correctly reg notes in LRA for multi-registers and set up biggest mode safely

Message ID 61a59ccf-229c-d86e-a6ec-8e6df26ec7d3@redhat.com
State New
Headers show
Series
  • [committed,PR99781] Update correctly reg notes in LRA for multi-registers and set up biggest mode safely
Related show

Commit Message

Paul Richard Thomas via Gcc-patches March 31, 2021, 9:06 p.m.
The following patch fixes

    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99781

The patch was successfully bootstrapped and tested on x86-64, ppc64le, 
and aarch64.

Patch

commit 1458059fc1faf6170f2fe45159065f91876307ac
Author: Vladimir N. Makarov <vmakarov@redhat.com>
Date:   Wed Mar 31 13:26:30 2021 -0400

    [PR99781] Update correctly reg notes in LRA for multi-registers and set up biggest mode safely
    
    The PR is about incorrect use of partial_subreg_p for unordered modes.
    I found 2 places of dangerous comparing unordered modes in LRA.  The
    patch removes dangerous use of paradoxical_subreg_p and
    partial_subreg_p in split_reg and process_bb_lives.  The both places
    used them to solve PR77761 long time ago.  But the problem was also
    fixed by later patches too (if there is no hard reg explicitly, it
    have VOIDmode and we use natural mode to split hard reg live,
    otherwise we use the biggest explicitly used mode for hard reg
    splitting).  The PR also says about inaccurate update of reg notes in
    LRA.  It happens for reg notes which refer for multi-registers.  The
    patch also fixes this issue.
    
    gcc/ChangeLog:
    
            PR target/99781
            * lra-constraints.c (split_reg): Don't check paradoxical_subreg_p.
            * lra-lives.c (clear_sparseset_regnos, regnos_in_sparseset_p): New
            functions.
            (process_bb_lives): Don't update biggest mode of hard reg for
            implicit in multi-register group.  Use the new functions for
            updating dead_set and unused_set by register notes.
    
    gcc/testsuite/ChangeLog:
    
            PR target/99781
            * g++.target/aarch64/sve/pr99781.C: New.

diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index 9993065f8d6..62bcfc31772 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -5796,12 +5796,11 @@  split_reg (bool before_p, int original_regno, rtx_insn *insn,
       nregs = 1;
       mode = lra_reg_info[hard_regno].biggest_mode;
       machine_mode reg_rtx_mode = GET_MODE (regno_reg_rtx[hard_regno]);
-      /* A reg can have a biggest_mode of VOIDmode if it was only ever seen
-	 as part of a multi-word register.  In that case, or if the biggest
-	 mode was larger than a register, just use the reg_rtx.  Otherwise,
-	 limit the size to that of the biggest access in the function.  */
-      if (mode == VOIDmode
-	  || paradoxical_subreg_p (mode, reg_rtx_mode))
+      /* A reg can have a biggest_mode of VOIDmode if it was only ever seen as
+	 part of a multi-word register.  In that case, just use the reg_rtx.
+	 Otherwise, limit the size to that of the biggest access in the
+	 function.  */
+      if (mode == VOIDmode)
 	{
 	  original_reg = regno_reg_rtx[hard_regno];
 	  mode = reg_rtx_mode;
diff --git a/gcc/lra-lives.c b/gcc/lra-lives.c
index 0bddca13fee..29531843c63 100644
--- a/gcc/lra-lives.c
+++ b/gcc/lra-lives.c
@@ -615,6 +615,32 @@  reg_early_clobber_p (const struct lra_insn_reg *reg, int n_alt)
 	     && TEST_BIT (reg->early_clobber_alts, n_alt)));
 }
 
+/* Clear pseudo REGNO in SET or all hard registers of REGNO in MODE in SET.  */
+static void
+clear_sparseset_regnos (sparseset set, int regno, enum machine_mode mode)
+{
+  if (regno >= FIRST_PSEUDO_REGISTER)
+    {
+      sparseset_clear_bit (dead_set, regno);
+      return;
+    }
+  for (int last = end_hard_regno (mode, regno); regno < last; regno++)
+    sparseset_clear_bit (set, regno);
+}
+
+/* Return true if pseudo REGNO is in SET or all hard registers of REGNO in MODE
+   are in SET.  */
+static bool
+regnos_in_sparseset_p (sparseset set, int regno, enum machine_mode mode)
+{
+  if (regno >= FIRST_PSEUDO_REGISTER)
+    return sparseset_bit_p (dead_set, regno);
+  for (int last = end_hard_regno (mode, regno); regno < last; regno++)
+    if (!sparseset_bit_p (set, regno))
+      return false;
+  return true;
+}
+
 /* Process insns of the basic block BB to update pseudo live ranges,
    pseudo hard register conflicts, and insn notes.  We do it on
    backward scan of BB insns.  CURR_POINT is the program point where
@@ -739,24 +765,13 @@  process_bb_lives (basic_block bb, int &curr_point, bool dead_insn_p)
       /* Update max ref width and hard reg usage.  */
       for (reg = curr_id->regs; reg != NULL; reg = reg->next)
 	{
-	  int i, regno = reg->regno;
+	  int regno = reg->regno;
 
 	  if (partial_subreg_p (lra_reg_info[regno].biggest_mode,
 				reg->biggest_mode))
 	    lra_reg_info[regno].biggest_mode = reg->biggest_mode;
 	  if (HARD_REGISTER_NUM_P (regno))
-	    {
-	      lra_hard_reg_usage[regno] += freq;
-	      /* A hard register explicitly can be used in small mode,
-		 but implicitly it can be used in natural mode as a
-		 part of multi-register group.  Process this case
-		 here.  */
-	      for (i = 1; i < hard_regno_nregs (regno, reg->biggest_mode); i++)
-		if (partial_subreg_p (lra_reg_info[regno + i].biggest_mode,
-				      GET_MODE (regno_reg_rtx[regno + i])))
-		  lra_reg_info[regno + i].biggest_mode
-		    = GET_MODE (regno_reg_rtx[regno + i]);
-	    }
+	    lra_hard_reg_usage[regno] += freq;
 	}
 
       call_p = CALL_P (curr_insn);
@@ -991,19 +1006,25 @@  process_bb_lives (basic_block bb, int &curr_point, bool dead_insn_p)
 	    ;
 	  else if (REG_P (XEXP (link, 0)))
 	    {
-	      regno = REGNO (XEXP (link, 0));
+	      rtx note_reg = XEXP (link, 0);
+	      int note_regno = REGNO (note_reg);
+
 	      if ((REG_NOTE_KIND (link) == REG_DEAD
-		   && ! sparseset_bit_p (dead_set, regno))
+		   && ! regnos_in_sparseset_p (dead_set, note_regno,
+					       GET_MODE (note_reg)))
 		  || (REG_NOTE_KIND (link) == REG_UNUSED
-		      && ! sparseset_bit_p (unused_set, regno)))
+		      && ! regnos_in_sparseset_p (unused_set, note_regno,
+						  GET_MODE (note_reg))))
 		{
 		  *link_loc = XEXP (link, 1);
 		  continue;
 		}
 	      if (REG_NOTE_KIND (link) == REG_DEAD)
-		sparseset_clear_bit (dead_set, regno);
+		clear_sparseset_regnos (dead_set, note_regno,
+					GET_MODE (note_reg));
 	      else if (REG_NOTE_KIND (link) == REG_UNUSED)
-		sparseset_clear_bit (unused_set, regno);
+		clear_sparseset_regnos (unused_set, note_regno,
+					GET_MODE (note_reg));
 	    }
 	  link_loc = &XEXP (link, 1);
 	}
diff --git a/gcc/testsuite/g++.target/aarch64/sve/pr99781.C b/gcc/testsuite/g++.target/aarch64/sve/pr99781.C
new file mode 100644
index 00000000000..21da8e04d8f
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/sve/pr99781.C
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=armv8-a+sve" } */
+
+#include <stdint.h>
+
+typedef int32_t vnx4si __attribute__((vector_size(32)));
+
+void
+foo (int32_t val)
+{
+  register vnx4si x asm ("z0");
+  register vnx4si y asm ("z1");
+  asm volatile ("" : "=w" (y));
+  val += 1;
+  vnx4si z = { val, val, val, val, val, val, val, val };
+  x = (vnx4si) { -1, 0, 0, -1, 0, -1, 0, -1 } ? z : y;
+  asm volatile ("" :: "w" (x));
+}