[PATCH] Fix PR88739

[PATCH] Fix PR88739

[Richard Biener]

This restricts BIT_FIELD_REFs we create to ones operating on
mode-precision entities (iff INTEGRAL_TYPE_P).  This avoids
issues with endianess when expanding them.

No testcase, BE folks have to come up with sth suitable for
the testsuite.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk,
queued for backporting (w/o the checking hunks).

Richard.

2019-01-28  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/88739
	* tree-cfg.c (verify_types_in_gimple_reference): Verify
	BIT_FIELD_REFs only are applied to mode-precision operands
	when they are integral.
	(verify_gimple_assign_ternary): Likewise for BIT_INSERT_EXPR.
	* tree-ssa-sccvn.c (vn_reference_lookup_3): Avoid generating
	BIT_FIELD_REFs of non-mode-precision integral operands.

Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c	(revision 268257)
+++ gcc/tree-cfg.c	(working copy)
@@ -3118,6 +3118,12 @@ verify_types_in_gimple_reference (tree e
 		     "match field size of BIT_FIELD_REF");
 	      return true;
 	    }
+	  if (INTEGRAL_TYPE_P (TREE_TYPE (op))
+	      && !type_has_mode_precision_p (TREE_TYPE (op)))
+	    {
+	      error ("BIT_FIELD_REF of non-mode-precision operand");
+	      return true;
+	    }
 	  if (!AGGREGATE_TYPE_P (TREE_TYPE (op))
 	      && maybe_gt (size + bitpos,
 			   tree_to_poly_uint64 (TYPE_SIZE (TREE_TYPE (op)))))
@@ -4319,6 +4325,12 @@ verify_gimple_assign_ternary (gassign *s
 	  error ("invalid position or size in BIT_INSERT_EXPR");
 	  return true;
 	}
+      if (INTEGRAL_TYPE_P (rhs1_type)
+	  && !type_has_mode_precision_p (rhs1_type))
+	{
+	  error ("BIT_INSERT_EXPR into non-mode-precision operand");
+	  return true;
+	}
       if (INTEGRAL_TYPE_P (rhs1_type))
 	{
 	  unsigned HOST_WIDE_INT bitpos = tree_to_uhwi (rhs3);
Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c	(revision 268257)
+++ gcc/tree-ssa-sccvn.c	(working copy)
@@ -2298,6 +2298,7 @@ vn_reference_lookup_3 (ao_ref *ref, tree
       base2 = get_ref_base_and_extent (gimple_assign_lhs (def_stmt),
 				       &offset2, &size2, &maxsize2,
 				       &reverse);
+      tree def_rhs = gimple_assign_rhs1 (def_stmt);
       if (!reverse
 	  && known_size_p (maxsize2)
 	  && known_eq (maxsize2, size2)
@@ -2309,11 +2310,13 @@ vn_reference_lookup_3 (ao_ref *ref, tree
 	     according to endianness.  */
 	  && (! INTEGRAL_TYPE_P (vr->type)
 	      || known_eq (ref->size, TYPE_PRECISION (vr->type)))
-	  && multiple_p (ref->size, BITS_PER_UNIT))
+	  && multiple_p (ref->size, BITS_PER_UNIT)
+	  && (! INTEGRAL_TYPE_P (TREE_TYPE (def_rhs))
+	      || type_has_mode_precision_p (TREE_TYPE (def_rhs))))
 	{
 	  gimple_match_op op (gimple_match_cond::UNCOND,
 			      BIT_FIELD_REF, vr->type,
-			      vn_valueize (gimple_assign_rhs1 (def_stmt)),
+			      vn_valueize (def_rhs),
 			      bitsize_int (ref->size),
 			      bitsize_int (offset - offset2));
 	  tree val = vn_nary_build_or_lookup (&op);

[PATCH] Fix PR88739

[Richard Biener]

The following fixes PR88739, VN figuring equivalences that later
lead to code insertions for code hoisting using a possibly overflowing
computation that wasn't done unconditionally before.

It is now possible to verify this with the new VN scheme.  Still
a more complete fix is pending (there's a related testcase and
the underlying issue is how PRE keeps track of expressions).

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

From 6d63150293c634cdea4e220a4163cf00f91318bf Mon Sep 17 00:00:00 2001
From: Richard Guenther <rguenther@suse.de>
Date: Tue, 8 Jan 2019 10:24:25 +0100
Subject: [PATCH] fix-pr86554

	PR tree-optimization/86554
	* tree-ssa-sccvn.c (eliminate_dom_walker, rpo_elim,
	rpo_avail): Move earlier.
	(visit_nary_op): When value-numbering to expressions
	with different overflow behavior make sure there's an
	available expression on the path.

	* gcc.dg/torture/pr86554-1.c: New testcase.
	* gcc.dg/torture/pr86554-2.c: Likewise.

diff --git a/gcc/testsuite/gcc.dg/torture/pr86554-1.c b/gcc/testsuite/gcc.dg/torture/pr86554-1.c
new file mode 100644
index 00000000000..64f851e896e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr86554-1.c
@@ -0,0 +1,35 @@
+/* { dg-do run } */
+
+struct foo
+{
+  unsigned x;
+};
+typedef struct foo foo;
+
+static inline int zot(foo *f)
+{
+  int ret;
+
+  if (f->x > 0x7FFFFFFF)
+    ret = (int)(f->x - 0x7FFFFFFF);
+  else
+    ret = (int)f->x - 0x7FFFFFFF;
+  return ret;
+}
+
+void __attribute__((noinline,noclone)) bar(foo *f)
+{
+  int ret = zot(f);
+  volatile int x = ret;
+  if (ret < 1)
+    __builtin_abort ();
+}
+
+int main()
+{
+  foo f;
+  f.x = 0x800003f8;
+
+  bar(&f);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/torture/pr86554-2.c b/gcc/testsuite/gcc.dg/torture/pr86554-2.c
new file mode 100644
index 00000000000..9e57a9ca725
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr86554-2.c
@@ -0,0 +1,49 @@
+/* { dg-do run } */
+/* { dg-require-effective-target int32plus } */
+
+struct s { __INT64_TYPE__ e; };
+
+static void f (struct s *ps)
+{
+  volatile __INT64_TYPE__ m = 9223372036854775807;
+  const char *str = "11E";
+  int r;
+  __INT64_TYPE__ sum;
+
+  ps->e = 0;
+
+  for (;;)
+    {
+      if (*str++ != '1')
+	break;
+      ps->e ++;
+    }
+
+  r = 1;
+  sum = m;
+
+  if (sum >= 0 && ps->e >= 0)
+    {
+      __UINT64_TYPE__ uc;
+      uc = (__UINT64_TYPE__) sum + (__UINT64_TYPE__) ps->e;
+      if (uc > 9223372036854775807)
+	r = 2;
+      else
+	sum = 17;
+    }
+  else
+    sum = sum + ps->e;
+
+  if (sum != 9223372036854775807)
+    __builtin_abort ();
+  if (r != 2)
+    __builtin_abort ();
+  ps->e = sum;
+}
+
+int main (void)
+{
+  struct s s;
+  f (&s);
+  return 0;
+}
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 2cebb6b37dc..ff54a66534e 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -1865,6 +1865,86 @@ vn_nary_simplify (vn_nary_op_t nary)
   return vn_nary_build_or_lookup_1 (&op, false);
 }
 
+/* Elimination engine.  */
+
+class eliminate_dom_walker : public dom_walker
+{
+public:
+  eliminate_dom_walker (cdi_direction, bitmap);
+  ~eliminate_dom_walker ();
+
+  virtual edge before_dom_children (basic_block);
+  virtual void after_dom_children (basic_block);
+
+  virtual tree eliminate_avail (basic_block, tree op);
+  virtual void eliminate_push_avail (basic_block, tree op);
+  tree eliminate_insert (basic_block, gimple_stmt_iterator *gsi, tree val);
+
+  void eliminate_stmt (basic_block, gimple_stmt_iterator *);
+
+  unsigned eliminate_cleanup (bool region_p = false);
+
+  bool do_pre;
+  unsigned int el_todo;
+  unsigned int eliminations;
+  unsigned int insertions;
+
+  /* SSA names that had their defs inserted by PRE if do_pre.  */
+  bitmap inserted_exprs;
+
+  /* Blocks with statements that have had their EH properties changed.  */
+  bitmap need_eh_cleanup;
+
+  /* Blocks with statements that have had their AB properties changed.  */
+  bitmap need_ab_cleanup;
+
+  /* Local state for the eliminate domwalk.  */
+  auto_vec<gimple *> to_remove;
+  auto_vec<gimple *> to_fixup;
+  auto_vec<tree> avail;
+  auto_vec<tree> avail_stack;
+};
+
+/* Adaptor to the elimination engine using RPO availability.  */
+
+class rpo_elim : public eliminate_dom_walker
+{
+public:
+  rpo_elim(basic_block entry_)
+    : eliminate_dom_walker (CDI_DOMINATORS, NULL), entry (entry_) {}
+  ~rpo_elim();
+
+  virtual tree eliminate_avail (basic_block, tree op);
+
+  virtual void eliminate_push_avail (basic_block, tree);
+
+  basic_block entry;
+  /* Instead of having a local availability lattice for each
+     basic-block and availability at X defined as union of
+     the local availabilities at X and its dominators we're
+     turning this upside down and track availability per
+     value given values are usually made available at very
+     few points (at least one).
+     So we have a value -> vec<location, leader> map where
+     LOCATION is specifying the basic-block LEADER is made
+     available for VALUE.  We push to this vector in RPO
+     order thus for iteration we can simply pop the last
+     entries.
+     LOCATION is the basic-block index and LEADER is its
+     SSA name version.  */
+  /* ???  We'd like to use auto_vec here with embedded storage
+     but that doesn't play well until we can provide move
+     constructors and use std::move on hash-table expansion.
+     So for now this is a bit more expensive than necessary.
+     We eventually want to switch to a chaining scheme like
+     for hashtable entries for unwinding which would make
+     making the vector part of the vn_ssa_aux structure possible.  */
+  typedef hash_map<tree, vec<std::pair<int, int> > > rpo_avail_t;
+  rpo_avail_t m_rpo_avail;
+};
+
+/* Global RPO state for access from hooks.  */
+static rpo_elim *rpo_avail;
 basic_block vn_context_bb;
 
 /* Callback for walk_non_aliased_vuses.  Tries to perform a lookup
@@ -3843,7 +3923,15 @@ visit_nary_op (tree lhs, gassign *stmt)
 		  ops[0] = vn_nary_op_lookup_pieces
 		      (2, gimple_assign_rhs_code (def), type, ops, NULL);
 		  /* We have wider operation available.  */
-		  if (ops[0])
+		  if (ops[0]
+		      /* If the leader is a wrapping operation we can
+		         insert it for code hoisting w/o introducing
+			 undefined overflow.  If it is not it has to
+			 be available.  See PR86554.  */
+		      && (TYPE_OVERFLOW_WRAPS (TREE_TYPE (ops[0]))
+			  || (rpo_avail && vn_context_bb
+			      && rpo_avail->eliminate_avail (vn_context_bb,
+							     ops[0]))))
 		    {
 		      unsigned lhs_prec = TYPE_PRECISION (type);
 		      unsigned rhs_prec = TYPE_PRECISION (TREE_TYPE (rhs1));
@@ -4654,45 +4742,6 @@ vn_nary_may_trap (vn_nary_op_t nary)
   return false;
 }
 
-
-class eliminate_dom_walker : public dom_walker
-{
-public:
-  eliminate_dom_walker (cdi_direction, bitmap);
-  ~eliminate_dom_walker ();
-
-  virtual edge before_dom_children (basic_block);
-  virtual void after_dom_children (basic_block);
-
-  virtual tree eliminate_avail (basic_block, tree op);
-  virtual void eliminate_push_avail (basic_block, tree op);
-  tree eliminate_insert (basic_block, gimple_stmt_iterator *gsi, tree val);
-
-  void eliminate_stmt (basic_block, gimple_stmt_iterator *);
-
-  unsigned eliminate_cleanup (bool region_p = false);
-
-  bool do_pre;
-  unsigned int el_todo;
-  unsigned int eliminations;
-  unsigned int insertions;
-
-  /* SSA names that had their defs inserted by PRE if do_pre.  */
-  bitmap inserted_exprs;
-
-  /* Blocks with statements that have had their EH properties changed.  */
-  bitmap need_eh_cleanup;
-
-  /* Blocks with statements that have had their AB properties changed.  */
-  bitmap need_ab_cleanup;
-
-  /* Local state for the eliminate domwalk.  */
-  auto_vec<gimple *> to_remove;
-  auto_vec<gimple *> to_fixup;
-  auto_vec<tree> avail;
-  auto_vec<tree> avail_stack;
-};
-
 eliminate_dom_walker::eliminate_dom_walker (cdi_direction direction,
 					    bitmap inserted_exprs_)
   : dom_walker (direction), do_pre (inserted_exprs_ != NULL),
@@ -5648,47 +5697,6 @@ free_rpo_vn (void)
   BITMAP_FREE (constant_value_ids);
 }
 
-/* Adaptor to the elimination engine using RPO availability.  */
-
-class rpo_elim : public eliminate_dom_walker
-{
-public:
-  rpo_elim(basic_block entry_)
-    : eliminate_dom_walker (CDI_DOMINATORS, NULL), entry (entry_) {}
-  ~rpo_elim();
-
-  virtual tree eliminate_avail (basic_block, tree op);
-
-  virtual void eliminate_push_avail (basic_block, tree);
-
-  basic_block entry;
-  /* Instead of having a local availability lattice for each
-     basic-block and availability at X defined as union of
-     the local availabilities at X and its dominators we're
-     turning this upside down and track availability per
-     value given values are usually made available at very
-     few points (at least one).
-     So we have a value -> vec<location, leader> map where
-     LOCATION is specifying the basic-block LEADER is made
-     available for VALUE.  We push to this vector in RPO
-     order thus for iteration we can simply pop the last
-     entries.
-     LOCATION is the basic-block index and LEADER is its
-     SSA name version.  */
-  /* ???  We'd like to use auto_vec here with embedded storage
-     but that doesn't play well until we can provide move
-     constructors and use std::move on hash-table expansion.
-     So for now this is a bit more expensive than necessary.
-     We eventually want to switch to a chaining scheme like
-     for hashtable entries for unwinding which would make
-     making the vector part of the vn_ssa_aux structure possible.  */
-  typedef hash_map<tree, vec<std::pair<int, int> > > rpo_avail_t;
-  rpo_avail_t m_rpo_avail;
-};
-
-/* Global RPO state for access from hooks.  */
-static rpo_elim *rpo_avail;
-
 /* Hook for maybe_push_res_to_seq, lookup the expression in the VN tables.  */
 
 static tree