cselib: Fix handling of multireg values for call insns [PR93170]
authorRichard Sandiford <richard.sandiford@arm.com>
Sat, 18 Jan 2020 10:59:10 +0000 (10:59 +0000)
committerRichard Sandiford <richard.sandiford@arm.com>
Mon, 27 Jan 2020 10:53:34 +0000 (10:53 +0000)
g:3bd2918594dae34ae84f 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:3bd2918594dae34ae84f 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
gcc/cselib.c
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/torture/pr93170.c [new file with mode: 0644]

index 36932a17a6d691e56863448d29e5617f70790b78..db2421d1c947e3f48f9cac6b1a6026d00b8fdab7 100644 (file)
@@ -1,3 +1,13 @@
+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
index 845f79b99ee045f074c6fa4f2609a011b5fa37db..3e0c69d67b81c1f95d0d0026a292ba97898d96bd 100644 (file)
@@ -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
index 22a37dd1ab217dd8e71ea1aca6da7077f6116f2c..16ddef07516a4d0579efcfb90137f499a3506d2f 100644 (file)
@@ -1,3 +1,7 @@
+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
diff --git a/gcc/testsuite/gcc.dg/torture/pr93170.c b/gcc/testsuite/gcc.dg/torture/pr93170.c
new file mode 100644 (file)
index 0000000..25a93a3
--- /dev/null
@@ -0,0 +1,33 @@
+/* { 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;
+}