public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-9215] tree-optimization/114121 - wrong VN with context sensitive range info
@ 2024-02-28 14:03 Richard Biener
  0 siblings, 0 replies; only message in thread
From: Richard Biener @ 2024-02-28 14:03 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:c841144a94363ff26e40ab3f26b14702c32987a8

commit r14-9215-gc841144a94363ff26e40ab3f26b14702c32987a8
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Feb 28 12:37:07 2024 +0100

    tree-optimization/114121 - wrong VN with context sensitive range info
    
    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/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/pr114121.c: New testcase.

Diff:
---
 gcc/testsuite/gcc.dg/torture/pr114121.c | 35 +++++++++++++++++++++
 gcc/tree-ssa-pre.cc                     |  5 +--
 gcc/tree-ssa-sccvn.cc                   | 55 ++++++++++++++++++++++++++++++---
 gcc/tree-ssa-sccvn.h                    |  3 ++
 4 files changed, 91 insertions(+), 7 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/torture/pr114121.c b/gcc/testsuite/gcc.dg/torture/pr114121.c
new file mode 100644
index 00000000000..9a6ddf2957e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr114121.c
@@ -0,0 +1,35 @@
+/* { dg-do run { target bitint } } */
+
+#if __BITINT_MAXWIDTH__ >= 256
+unsigned a, b, c, d, e;
+unsigned _BitInt(256) f;
+
+__attribute__((noipa)) unsigned short
+bswap16 (int t)
+{
+  return __builtin_bswap16 (t);
+}
+
+void
+foo (unsigned z, unsigned _BitInt(512) y, unsigned *r)
+{
+  unsigned t = __builtin_sub_overflow_p (0, y << 509, f);
+  z *= bswap16 (t);
+  d = __builtin_sub_overflow_p (c, 3, (unsigned _BitInt(512)) 0);
+  unsigned q = z + c + b;
+  unsigned short n = q >> (8 + a);
+  *r = b + e + n;
+}
+#endif
+
+int
+main ()
+{
+#if __BITINT_MAXWIDTH__ >= 256
+  unsigned x;
+  foo (8, 2, &x);
+  if (x != 8)
+    __builtin_abort ();
+#endif
+  return 0;
+}
diff --git a/gcc/tree-ssa-pre.cc b/gcc/tree-ssa-pre.cc
index d29214d04f8..75217f5cde1 100644
--- a/gcc/tree-ssa-pre.cc
+++ b/gcc/tree-ssa-pre.cc
@@ -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;
 	      }
diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc
index 2587eb1c505..a10c00b8985 100644
--- a/gcc/tree-ssa-sccvn.cc
+++ b/gcc/tree-ssa-sccvn.cc
@@ -438,7 +438,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.  */
@@ -748,6 +748,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)
@@ -772,6 +774,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;
@@ -2061,6 +2073,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);
 }
 
@@ -2537,6 +2550,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,
@@ -2550,15 +2565,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
@@ -3756,6 +3774,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;
@@ -3944,6 +3964,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;
@@ -4008,6 +4030,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;
@@ -4134,6 +4160,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))
@@ -4156,7 +4184,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);
@@ -4212,6 +4246,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);
 }
@@ -4275,6 +4311,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;
@@ -4317,7 +4357,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)
 
@@ -4334,6 +4375,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);
@@ -5852,6 +5895,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;
diff --git a/gcc/tree-ssa-sccvn.h b/gcc/tree-ssa-sccvn.h
index 8ec1de02d30..82f6f7323f9 100644
--- a/gcc/tree-ssa-sccvn.h
+++ b/gcc/tree-ssa-sccvn.h
@@ -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>);

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2024-02-28 14:03 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-28 14:03 [gcc r14-9215] tree-optimization/114121 - wrong VN with context sensitive range info Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).