]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
tree-optimization/114121 - wrong VN with context sensitive range info
authorRichard Biener <rguenther@suse.de>
Wed, 28 Feb 2024 11:37:07 +0000 (12:37 +0100)
committerRichard Biener <rguenther@suse.de>
Tue, 7 May 2024 06:18:38 +0000 (08:18 +0200)
When VN ends up exploiting range-info specifying the ao_ref offset
and max_size we have to make sure to reflect this in the hashtable
entry for the recorded expression.  The PR113831 fix handled the
case where we can encode this in the operands themselves but this
bug shows the issue is more widespread.

So instead of altering the operands the following instead records
this extra info that's possibly used, only throwing it away when
the value-numbering didn't come up with a non-VARYING value which
is an important detail to preserve CSE as opposed to constant
folding which is where all cases currently known popped up.

With this the original PR113831 fix can be reverted.

PR tree-optimization/113831
PR tree-optimization/114121
* tree-ssa-sccvn.h (vn_reference_s::offset,
vn_reference_s::max_size): New fields.
(vn_reference_insert_pieces): Adjust prototype.
* tree-ssa-pre.cc (phi_translate_1): Preserve offset/max_size.
* tree-ssa-sccvn.cc (vn_reference_eq): Compare offset and
size, allow using "don't know" state.
(vn_walk_cb_data::finish): Pass along offset/max_size.
(vn_reference_lookup_or_insert_for_pieces): Take offset and
max_size as argument and use it.
(vn_reference_lookup_3): Properly adjust offset and max_size
according to the adjusted ao_ref.
(vn_reference_lookup_pieces): Initialize offset and max_size.
(vn_reference_lookup): Likewise.
(vn_reference_lookup_call): Likewise.
(vn_reference_insert): Likewise.
(visit_reference_op_call): Likewise.
(vn_reference_insert_pieces): Take offset and max_size
as argument and use it.

* gcc.dg/torture/pr113831.c: New testcase.

(cherry picked from commit c841144a94363ff26e40ab3f26b14702c32987a8)

gcc/testsuite/gcc.dg/torture/pr113831.c [new file with mode: 0644]
gcc/tree-ssa-pre.cc
gcc/tree-ssa-sccvn.cc
gcc/tree-ssa-sccvn.h

diff --git a/gcc/testsuite/gcc.dg/torture/pr113831.c b/gcc/testsuite/gcc.dg/torture/pr113831.c
new file mode 100644 (file)
index 0000000..12aea7c
--- /dev/null
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+
+int a[3];
+int __attribute__((noipa))
+foo(int i, int x)
+{
+  int tem = 0;
+  a[2] = x;
+  if (i < 1)
+    ++i;
+  else
+    {
+      ++i;
+      tem = a[i];
+    }
+  tem += a[i];
+  return tem;
+}
+
+int
+main()
+{
+  if (foo (0, 7) != 0)
+    __builtin_abort();
+  return 0;
+}
index 9e9a2f9d26aaf3864d2b1ad681dd012a153e5033..b570e9e26f0254731447ddc9da868d6c955540c7 100644 (file)
@@ -1666,8 +1666,9 @@ phi_translate_1 (bitmap_set_t dest,
                if (!newoperands.exists ())
                  newoperands = operands.copy ();
                newref = vn_reference_insert_pieces (newvuse, ref->set,
-                                                    ref->base_set, ref->type,
-                                                    newoperands,
+                                                    ref->base_set,
+                                                    ref->offset, ref->max_size,
+                                                    ref->type, newoperands,
                                                     result, new_val_id);
                newoperands = vNULL;
              }
index b20a002a14466caca475c23c2c0377dbc2e50917..fe4fc256deaf8c5e9b6146a2219b7d72e4bd062f 100644 (file)
@@ -432,7 +432,7 @@ static void init_vn_nary_op_from_pieces (vn_nary_op_t, unsigned int,
                                         enum tree_code, tree, tree *);
 static tree vn_lookup_simplify_result (gimple_match_op *);
 static vn_reference_t vn_reference_lookup_or_insert_for_pieces
-         (tree, alias_set_type, alias_set_type, tree,
+         (tree, alias_set_type, alias_set_type, poly_int64, poly_int64, tree,
           vec<vn_reference_op_s, va_heap>, tree);
 
 /* Return whether there is value numbering information for a given SSA name.  */
@@ -742,6 +742,8 @@ vn_reference_compute_hash (const vn_reference_t vr1)
            vn_reference_op_compute_hash (vro, hstate);
        }
     }
+  /* Do not hash vr1->offset or vr1->max_size, we want to get collisions
+     to be able to identify compatible results.  */
   result = hstate.end ();
   /* ??? We would ICE later if we hash instead of adding that in. */
   if (vr1->vuse)
@@ -766,6 +768,16 @@ vn_reference_eq (const_vn_reference_t const vr1, const_vn_reference_t const vr2)
   if (vr1->vuse != vr2->vuse)
     return false;
 
+  /* The offset/max_size used for the ao_ref during lookup has to be
+     the same.  */
+  if (maybe_ne (vr1->offset, vr2->offset)
+      || maybe_ne (vr1->max_size, vr2->max_size))
+    {
+      /* But nothing known in the prevailing entry is OK to be used.  */
+      if (maybe_ne (vr1->offset, 0) || known_size_p (vr1->max_size))
+       return false;
+    }
+
   /* If the operands are the same we are done.  */
   if (vr1->operands == vr2->operands)
     return true;
@@ -1909,6 +1921,7 @@ vn_walk_cb_data::finish (alias_set_type set, alias_set_type base_set, tree val)
   vec<vn_reference_op_s> &operands
     = saved_operands.exists () ? saved_operands : vr->operands;
   return vn_reference_lookup_or_insert_for_pieces (last_vuse, set, base_set,
+                                                  vr->offset, vr->max_size,
                                                   vr->type, operands, val);
 }
 
@@ -2352,6 +2365,8 @@ static vn_reference_t
 vn_reference_lookup_or_insert_for_pieces (tree vuse,
                                          alias_set_type set,
                                          alias_set_type base_set,
+                                         poly_int64 offset,
+                                         poly_int64 max_size,
                                          tree type,
                                          vec<vn_reference_op_s,
                                                va_heap> operands,
@@ -2365,15 +2380,18 @@ vn_reference_lookup_or_insert_for_pieces (tree vuse,
   vr1.type = type;
   vr1.set = set;
   vr1.base_set = base_set;
+  vr1.offset = offset;
+  vr1.max_size = max_size;
   vr1.hashcode = vn_reference_compute_hash (&vr1);
   if (vn_reference_lookup_1 (&vr1, &result))
     return result;
+
   if (TREE_CODE (value) == SSA_NAME)
     value_id = VN_INFO (value)->value_id;
   else
     value_id = get_or_alloc_constant_value_id (value);
-  return vn_reference_insert_pieces (vuse, set, base_set, type,
-                                    operands.copy (), value, value_id);
+  return vn_reference_insert_pieces (vuse, set, base_set, offset, max_size,
+                                    type, operands.copy (), value, value_id);
 }
 
 /* Return a value-number for RCODE OPS... either by looking up an existing
@@ -3557,6 +3575,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *data_,
            return (void *)-1;
        }
       *ref = r;
+      vr->offset = r.offset;
+      vr->max_size = r.max_size;
 
       /* Do not update last seen VUSE after translating.  */
       data->last_vuse_ptr = NULL;
@@ -3745,6 +3765,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *data_,
       if (maybe_ne (ref->size, r.size))
        return (void *)-1;
       *ref = r;
+      vr->offset = r.offset;
+      vr->max_size = r.max_size;
 
       /* Do not update last seen VUSE after translating.  */
       data->last_vuse_ptr = NULL;
@@ -3809,6 +3831,10 @@ vn_reference_lookup_pieces (tree vuse, alias_set_type set,
   vr1.type = type;
   vr1.set = set;
   vr1.base_set = base_set;
+  /* We can pretend there's no extra info fed in since the ao_refs offset
+     and max_size are computed only from the VN reference ops.  */
+  vr1.offset = 0;
+  vr1.max_size = -1;
   vr1.hashcode = vn_reference_compute_hash (&vr1);
   if ((cst = fully_constant_vn_reference_p (&vr1)))
     return cst;
@@ -3935,6 +3961,8 @@ vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind,
   ao_ref_init (&op_ref, op);
   vr1.set = ao_ref_alias_set (&op_ref);
   vr1.base_set = ao_ref_base_alias_set (&op_ref);
+  vr1.offset = 0;
+  vr1.max_size = -1;
   vr1.hashcode = vn_reference_compute_hash (&vr1);
   if (mask == NULL_TREE)
     if (tree cst = fully_constant_vn_reference_p (&vr1))
@@ -3957,7 +3985,13 @@ vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind,
       if (!valueized_anything
          || !ao_ref_init_from_vn_reference (&r, vr1.set, vr1.base_set,
                                             vr1.type, ops_for_ref))
-       ao_ref_init (&r, op);
+       {
+         ao_ref_init (&r, op);
+         /* Record the extra info we're getting from the full ref.  */
+         ao_ref_base (&r);
+         vr1.offset = r.offset;
+         vr1.max_size = r.max_size;
+       }
       vn_walk_cb_data data (&vr1, r.ref ? NULL_TREE : op,
                            last_vuse_ptr, kind, tbaa_p, mask,
                            redundant_store_removal_p);
@@ -4013,6 +4047,8 @@ vn_reference_lookup_call (gcall *call, vn_reference_t *vnresult,
   vr->punned = false;
   vr->set = 0;
   vr->base_set = 0;
+  vr->offset = 0;
+  vr->max_size = -1;
   vr->hashcode = vn_reference_compute_hash (vr);
   vn_reference_lookup_1 (vr, vnresult);
 }
@@ -4076,6 +4112,10 @@ vn_reference_insert (tree op, tree result, tree vuse, tree vdef)
   ao_ref_init (&op_ref, op);
   vr1->set = ao_ref_alias_set (&op_ref);
   vr1->base_set = ao_ref_base_alias_set (&op_ref);
+  /* Specifically use an unknown extent here, we're not doing any lookup
+     and assume the caller didn't either (or it went VARYING).  */
+  vr1->offset = 0;
+  vr1->max_size = -1;
   vr1->hashcode = vn_reference_compute_hash (vr1);
   vr1->result = TREE_CODE (result) == SSA_NAME ? SSA_VAL (result) : result;
   vr1->result_vdef = vdef;
@@ -4118,7 +4158,8 @@ vn_reference_insert (tree op, tree result, tree vuse, tree vdef)
 
 vn_reference_t
 vn_reference_insert_pieces (tree vuse, alias_set_type set,
-                           alias_set_type base_set, tree type,
+                           alias_set_type base_set,
+                           poly_int64 offset, poly_int64 max_size, tree type,
                            vec<vn_reference_op_s> operands,
                            tree result, unsigned int value_id)
 
@@ -4135,6 +4176,8 @@ vn_reference_insert_pieces (tree vuse, alias_set_type set,
   vr1->punned = false;
   vr1->set = set;
   vr1->base_set = base_set;
+  vr1->offset = offset;
+  vr1->max_size = max_size;
   vr1->hashcode = vn_reference_compute_hash (vr1);
   if (result && TREE_CODE (result) == SSA_NAME)
     result = SSA_VAL (result);
@@ -5652,6 +5695,8 @@ visit_reference_op_call (tree lhs, gcall *stmt)
       vr2->type = vr1.type;
       vr2->punned = vr1.punned;
       vr2->set = vr1.set;
+      vr2->offset = vr1.offset;
+      vr2->max_size = vr1.max_size;
       vr2->base_set = vr1.base_set;
       vr2->hashcode = vr1.hashcode;
       vr2->result = lhs;
index 675240ebb9d369eeaed33108119c0e1c90bb8d53..79e89ccdb437c2446ea992f55e05157037223744 100644 (file)
@@ -145,6 +145,8 @@ typedef struct vn_reference_s
   tree vuse;
   alias_set_type set;
   alias_set_type base_set;
+  poly_int64 offset;
+  poly_int64 max_size;
   tree type;
   unsigned punned : 1;
   vec<vn_reference_op_s> operands;
@@ -268,6 +270,7 @@ tree vn_reference_lookup (tree, tree, vn_lookup_kind, vn_reference_t *, bool,
                          tree * = NULL, tree = NULL_TREE, bool = false);
 void vn_reference_lookup_call (gcall *, vn_reference_t *, vn_reference_t);
 vn_reference_t vn_reference_insert_pieces (tree, alias_set_type, alias_set_type,
+                                          poly_int64, poly_int64,
                                           tree, vec<vn_reference_op_s>,
                                           tree, unsigned int);
 void print_vn_reference_ops (FILE *, const vec<vn_reference_op_s>);