Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)

[Marek Polacek <polacek at redhat dot com>]

On Thu, Feb 08, 2018 at 10:15:45AM +0000, Richard Sandiford wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> > On Wed, Feb 07, 2018 at 03:23:25PM -0500, Jason Merrill wrote:
> >> On Wed, Feb 7, 2018 at 2:48 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> >> > On Wed, Feb 07, 2018 at 08:36:31PM +0100, Marek Polacek wrote:
> >> >> > > That was my first patch, but it was rejected:
> >> >> > > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00271.html
> >> >> >
> >> >> > Then should we update fold_indirect_ref_1 to use the new code?  Is
> >> >> > there a reason for them to stay out of sync?
> >> >>
> >> >> One of the reasons is that middle end uses poly_uint64 type but the
> >> >> front ends
> >> >> shouldn't use them.  So some of these functions will unfortunately differ.
> >> >
> >> > Yeah.  Part of the patch makes the two implementations slightly more
> >> > similar, but I have e.g. no idea how to test for poly_uint64 that fits
> >> > also in poly_int64 and the poly_int* stuff makes the two substantially
> >> > different in any case.
> >> 
> >> Hmm.  Well, that seems rather unfortunate.  Why shouldn't the front
> >> ends use them?  Can we make an exception for this function because
> >> it's supposed to mirror a middle-end function?
> >> Should we try to push this function back into the middle end?
> >
> > The function comment seems to explain the reasons:
> > /* A less strict version of fold_indirect_ref_1, which requires cv-quals to
> >    match.  We want to be less strict for simple *& folding; if we have a
> >    non-const temporary that we access through a const pointer, that should
> >    work.  We handle this here rather than change fold_indirect_ref_1
> >    because we're dealing with things like ADDR_EXPR of INTEGER_CST which
> >    don't really make sense outside of constant expression evaluation.  Also
> >    we want to allow folding to COMPONENT_REF, which could cause trouble
> >    with TBAA in fold_indirect_ref_1.
> >    
> >    Try to keep this function synced with fold_indirect_ref_1.  */
> >
> > E.g. the constexpr function uses same_type_ignoring_top_level_qualifiers_p
> > instead of == type comparisons, the COMPONENT_REF stuff, ...
> >
> > For poly_* stuff, I think Richard S. wants to introduce it into the FEs at
> > some point, but I could be wrong; certainly it hasn't been done yet and
> > generally, poly*int seems to be a nightmare to deal with.
> 
> There's no problem with FEs using poly_int now if they want to,
> or if it happens to be more convenient in a particular context
> (which seems to be the case here to avoid divergence).  It's more
> that FEs don't need to go out of their way to handle poly_int,
> since the FE can't yet introduce any cases in which the poly_ints
> would be nonconstant.
> 
> In practice FEs do already use poly_int directly (and indirectly
> via support routines).

In that case, this patch attemps to synchronize cxx_fold_indirect_ref and
fold_indirect_ref_1 somewhat, especially regarding the poly* stuff.

It also changes if-else-if to if, if, but I can drop that change.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-02-08  Marek Polacek  <polacek@redhat.com>

	PR c++/83659
	* constexpr.c (cxx_fold_indirect_ref): Synchronize with
	fold_indirect_ref_1.

	* g++.dg/torture/pr83659.C: New test.

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 93dd8ae049c..6286c7828c6 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -3025,9 +3025,10 @@ cxx_eval_vec_init (const constexpr_ctx *ctx, tree t,
 static tree
 cxx_fold_indirect_ref (location_t loc, tree type, tree op0, bool *empty_base)
 {
-  tree sub, subtype;
+  tree sub = op0;
+  tree subtype;
+  poly_uint64 const_op01;
 
-  sub = op0;
   STRIP_NOPS (sub);
   subtype = TREE_TYPE (sub);
   if (!POINTER_TYPE_P (subtype))
@@ -3106,8 +3107,9 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree op0, bool *empty_base)
 	      return fold_build3 (COMPONENT_REF, type, op, field, NULL_TREE);
 	}
     }
-  else if (TREE_CODE (sub) == POINTER_PLUS_EXPR
-	   && TREE_CODE (TREE_OPERAND (sub, 1)) == INTEGER_CST)
+
+  if (TREE_CODE (sub) == POINTER_PLUS_EXPR
+      && poly_int_tree_p (TREE_OPERAND (sub, 1), &const_op01))
     {
       tree op00 = TREE_OPERAND (sub, 0);
       tree op01 = TREE_OPERAND (sub, 1);
@@ -3124,26 +3126,25 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree op0, bool *empty_base)
 	      && (same_type_ignoring_top_level_qualifiers_p
 		  (type, TREE_TYPE (op00type))))
 	    {
-	      HOST_WIDE_INT offset = tree_to_shwi (op01);
 	      tree part_width = TYPE_SIZE (type);
-	      unsigned HOST_WIDE_INT part_widthi = tree_to_shwi (part_width)/BITS_PER_UNIT;
-	      unsigned HOST_WIDE_INT indexi = offset * BITS_PER_UNIT;
-	      tree index = bitsize_int (indexi);
-
-	      if (known_lt (offset / part_widthi,
-			    TYPE_VECTOR_SUBPARTS (op00type)))
-		return fold_build3_loc (loc,
-					BIT_FIELD_REF, type, op00,
-					part_width, index);
-
+	      poly_uint64 max_offset
+		= (tree_to_uhwi (part_width) / BITS_PER_UNIT
+		   * TYPE_VECTOR_SUBPARTS (op00type));
+	      if (known_lt (const_op01, max_offset))
+		{
+		  tree index = bitsize_int (const_op01 * BITS_PER_UNIT);
+		  return fold_build3_loc (loc,
+					  BIT_FIELD_REF, type, op00,
+					  part_width, index);
+		}
 	    }
 	  /* ((foo*)&complexfoo)[1] => __imag__ complexfoo */
 	  else if (TREE_CODE (op00type) == COMPLEX_TYPE
 		   && (same_type_ignoring_top_level_qualifiers_p
 		       (type, TREE_TYPE (op00type))))
 	    {
-	      tree size = TYPE_SIZE_UNIT (type);
-	      if (tree_int_cst_equal (size, op01))
+	      if (known_eq (wi::to_poly_offset (TYPE_SIZE_UNIT (type)),
+			    const_op01))
 		return fold_build1_loc (loc, IMAGPART_EXPR, type, op00);
 	    }
 	  /* ((foo *)&fooarray)[1] => fooarray[1] */
@@ -3191,10 +3192,11 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree op0, bool *empty_base)
 	    }
 	}
     }
+
   /* *(foo *)fooarrptr => (*fooarrptr)[0] */
-  else if (TREE_CODE (TREE_TYPE (subtype)) == ARRAY_TYPE
-	   && (same_type_ignoring_top_level_qualifiers_p
-	       (type, TREE_TYPE (TREE_TYPE (subtype)))))
+  if (TREE_CODE (TREE_TYPE (subtype)) == ARRAY_TYPE
+      && (same_type_ignoring_top_level_qualifiers_p
+	  (type, TREE_TYPE (TREE_TYPE (subtype)))))
     {
       tree type_domain;
       tree min_val = size_zero_node;
diff --git gcc/testsuite/g++.dg/torture/pr83659.C gcc/testsuite/g++.dg/torture/pr83659.C
index e69de29bb2d..d9f709bb520 100644
--- gcc/testsuite/g++.dg/torture/pr83659.C
+++ gcc/testsuite/g++.dg/torture/pr83659.C
@@ -0,0 +1,18 @@
+// PR c++/83659
+// { dg-do compile }
+
+typedef int V __attribute__ ((__vector_size__ (16)));
+V a;
+V b[2];
+
+int
+foo ()
+{
+  return reinterpret_cast <int *> (&a)[-1] += 1;
+}
+
+int
+bar ()
+{
+  return reinterpret_cast <int *> (&a[1])[-1];
+}

	Marek