cselib: Fix handling of multireg values for call insns [PR93170]

g:3bd29185 mishandled the case in which only the
tail end of a multireg hard register is invalidated by the call.
Walking all the entries should be both safer and more precise.

Avoiding cselib_invalidate_regno also means that we no longer
walk the same list multiple times (which is something we did
before g:3bd29185 too).

2020-01-27  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	PR rtl-optimization/93170
	* cselib.c (cselib_invalidate_regno_val): New function, split out
	from...
	(cselib_invalidate_regno): ...here.
	(cselib_invalidated_by_call_p): New function.
	(cselib_process_insn): Iterate over all the hard-register entries in
	REG_VALUES and invalidate any that cross call-clobbered registers.

gcc/testsuite/
	* gcc.dg/torture/pr93170.c: New test.

gcc/ChangeLog

 2020-01-27  Richard Sandiford  <richard.sandiford@arm.com>
 
+	PR rtl-optimization/93170
+	* cselib.c (cselib_invalidate_regno_val): New function, split out
+	from...
+	(cselib_invalidate_regno): ...here.
+	(cselib_invalidated_by_call_p): New function.
+	(cselib_process_insn): Iterate over all the hard-register entries in
+	REG_VALUES and invalidate any that cross call-clobbered registers.
+
+2020-01-27  Richard Sandiford  <richard.sandiford@arm.com>
+
 	* dojump.c (split_comparison): Use HONOR_NANS rather than
 	HONOR_SNANS when splitting LTGT.
 

gcc/cselib.c

@@ -2156,6 +2156,52 @@ cselib_lookup (rtx x, machine_mode mode,
   return ret;
 }
 
+/* Invalidate the value at *L, which is part of REG_VALUES (REGNO).  */
+
+static void
+cselib_invalidate_regno_val (unsigned int regno, struct elt_list **l)
+{
+  cselib_val *v = (*l)->elt;
+  if (*l == REG_VALUES (regno))
+    {
+      /* Maintain the invariant that the first entry of
+	 REG_VALUES, if present, must be the value used to set
+	 the register, or NULL.  This is also nice because
+	 then we won't push the same regno onto user_regs
+	 multiple times.  */
+      (*l)->elt = NULL;
+      l = &(*l)->next;
+    }
+  else
+    unchain_one_elt_list (l);
+
+  v = canonical_cselib_val (v);
+
+  bool had_locs = v->locs != NULL;
+  rtx_insn *setting_insn = v->locs ? v->locs->setting_insn : NULL;
+
+  /* Now, we clear the mapping from value to reg.  It must exist, so
+     this code will crash intentionally if it doesn't.  */
+  for (elt_loc_list **p = &v->locs; ; p = &(*p)->next)
+    {
+      rtx x = (*p)->loc;
+
+      if (REG_P (x) && REGNO (x) == regno)
+	{
+	  unchain_one_elt_loc_list (p);
+	  break;
+	}
+    }
+
+  if (had_locs && v->locs == 0 && !PRESERVED_VALUE_P (v->val_rtx))
+    {
+      if (setting_insn && DEBUG_INSN_P (setting_insn))
+	n_useless_debug_values++;
+      else
+	n_useless_values++;
+    }
+}
+
 /* Invalidate any entries in reg_values that overlap REGNO.  This is called
    if REGNO is changing.  MODE is the mode of the assignment to REGNO, which
    is used to determine how many hard registers are being changed.  If MODE
@@ -2202,9 +2248,6 @@ cselib_invalidate_regno (unsigned int regno, machine_mode mode)
       while (*l)
 	{
 	  cselib_val *v = (*l)->elt;
-	  bool had_locs;
-	  rtx_insn *setting_insn;
-	  struct elt_loc_list **p;
 	  unsigned int this_last = i;
 
 	  if (i < FIRST_PSEUDO_REGISTER && v != NULL)
@@ -2219,44 +2262,7 @@ cselib_invalidate_regno (unsigned int regno, machine_mode mode)
 	    }
 
 	  /* We have an overlap.  */
-	  if (*l == REG_VALUES (i))
-	    {
-	      /* Maintain the invariant that the first entry of
-		 REG_VALUES, if present, must be the value used to set
-		 the register, or NULL.  This is also nice because
-		 then we won't push the same regno onto user_regs
-		 multiple times.  */
-	      (*l)->elt = NULL;
-	      l = &(*l)->next;
-	    }
-	  else
-	    unchain_one_elt_list (l);
-
-	  v = canonical_cselib_val (v);
-
-	  had_locs = v->locs != NULL;
-	  setting_insn = v->locs ? v->locs->setting_insn : NULL;
-
-	  /* Now, we clear the mapping from value to reg.  It must exist, so
-	     this code will crash intentionally if it doesn't.  */
-	  for (p = &v->locs; ; p = &(*p)->next)
-	    {
-	      rtx x = (*p)->loc;
-
-	      if (REG_P (x) && REGNO (x) == i)
-		{
-		  unchain_one_elt_loc_list (p);
-		  break;
-		}
-	    }
-
-	  if (had_locs && v->locs == 0 && !PRESERVED_VALUE_P (v->val_rtx))
-	    {
-	      if (setting_insn && DEBUG_INSN_P (setting_insn))
-		n_useless_debug_values++;
-	      else
-		n_useless_values++;
-	    }
+	  cselib_invalidate_regno_val (i, l);
 	}
     }
 }
@@ -2714,6 +2720,28 @@ fp_setter_insn (rtx_insn *insn)
   return true;
 }
 
+/* V is one of the values in REG_VALUES (REGNO).  Return true if it
+   would be invalidated by CALLEE_ABI.  */
+
+static bool
+cselib_invalidated_by_call_p (const function_abi &callee_abi,
+			      unsigned int regno, cselib_val *v)
+{
+  machine_mode mode = GET_MODE (v->val_rtx);
+  if (mode == VOIDmode)
+    {
+      v = REG_VALUES (regno)->elt;
+      if (!v)
+	/* If we don't know what the mode of the constant value is, and we
+	   don't know what mode the register was set in, conservatively
+	   assume that the register is clobbered.  The value's going to be
+	   essentially useless in this case anyway.  */
+	return true;
+      mode = GET_MODE (v->val_rtx);
+    }
+  return callee_abi.clobbers_reg_p (mode, regno);
+}
+
 /* Record the effects of INSN.  */
 
 void
@@ -2748,24 +2776,17 @@ cselib_process_insn (rtx_insn *insn)
     {
       function_abi callee_abi = insn_callee_abi (insn);
       for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
-	if (elt_list *values = REG_VALUES (i))
-	  {
-	    /* If we know what mode the value was set in, check whether
-	       it is still available after the call in that mode.  If we
-	       don't know the mode, we have to check for the worst-case
-	       scenario instead.  */
-	    if (values->elt)
-	      {
-		machine_mode mode = GET_MODE (values->elt->val_rtx);
-		if (callee_abi.clobbers_reg_p (mode, i))
-		  cselib_invalidate_regno (i, mode);
-	      }
-	    else
-	      {
-		if (callee_abi.clobbers_at_least_part_of_reg_p (i))
-		  cselib_invalidate_regno (i, reg_raw_mode[i]);
-	      }
-	  }
+	{
+	  elt_list **l = &REG_VALUES (i);
+	  while (*l)
+	    {
+	      cselib_val *v = (*l)->elt;
+	      if (v && cselib_invalidated_by_call_p (callee_abi, i, v))
+		cselib_invalidate_regno_val (i, l);
+	      else
+		l = &(*l)->next;
+	    }
+	}
 
       /* Since it is not clear how cselib is going to be used, be
 	 conservative here and treat looping pure or const functions

gcc/testsuite/ChangeLog

+2020-01-27  Richard Sandiford  <richard.sandiford@arm.com>
+
+	* gcc.dg/torture/pr93170.c: New test.
+
 2020-01-27  Martin Liska  <mliska@suse.cz>
 
 	PR target/93274

gcc/testsuite/gcc.dg/torture/pr93170.c

+/* { dg-do run } */
+/* { dg-require-effective-target int128 } */
+/* { dg-additional-options "-frename-registers -fno-tree-forwprop -fno-tree-fre -fira-algorithm=priority -mstringop-strategy=loop --param=hot-bb-frequency-fraction=0 -Wno-psabi" { target { x86_64-*-* i?86-*-* } } } */
+
+typedef unsigned char v64u8 __attribute__ ((vector_size (64)));
+typedef unsigned short v64u16 __attribute__ ((vector_size (64)));
+typedef unsigned int v64u32 __attribute__ ((vector_size (64)));
+typedef unsigned long long v64u64 __attribute__ ((vector_size (64)));
+typedef unsigned __int128 u128;
+typedef unsigned __int128 v64u128 __attribute__ ((vector_size (64)));
+
+int a, b, d, e;
+v64u64 c;
+
+v64u128
+foo (u128 g, v64u16 h, v64u32 i, v64u128 j)
+{
+  c[e] = 0;
+  j &= (i[1] <<= b);
+  j >>= ((v64u128) h <= j);
+  d = __builtin_popcountll (-((v64u8) i)[0]);
+  return a + g + j;
+}
+
+int
+main (void)
+{
+  v64u128 x = foo (0, (v64u16) { 0, 0, 0, 0, 0, 0, 0, 0, 5 }, (v64u32) { 2 },
+		   (v64u128) { });
+  if (x[0] || x[1] || x[2] || x[3])
+    __builtin_abort ();
+  return 0;
+}