[PATCH] Fix ICE in store-merging (PR tree-optimization/82929)

Jakub Jelinek jakub@redhat.com
Fri Nov 10 13:59:00 GMT 2017


Hi!

Because BIT_{AND,IOR,XOR}_EXPR are commutative, we std::swap the
store_operand_info ops if that means we could append the store into
a group rather than having to start a new group.  Unfortunately
for count_multiple_uses we need to walk the stmts computing the stored value
in order to check the has_single_use stuff and if we've swapped the
arguments, that confuses the function.

This patch just records that we've swapped them and then the function
can take that into account easily.

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

2017-11-10  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/82929
	* gimple-ssa-store-merging.c (struct store_immediate_info): Add
	ops_swapped_p non-static data member.
	(store_immediate_info::store_immediate_info): Clear it.
	(imm_store_chain_info::coalesce_immediate_stores): If swapping
	ops set ops_swapped_p.
	(count_multiple_uses): Handle ops_swapped_p.

	* gcc.dg/pr82929.c: New test.
	* g++.dg/opt/pr82929.C: New test.

--- gcc/gimple-ssa-store-merging.c.jj	2017-11-09 20:24:34.000000000 +0100
+++ gcc/gimple-ssa-store-merging.c	2017-11-10 08:04:49.192744048 +0100
@@ -209,7 +209,11 @@ struct store_immediate_info
   /* INTEGER_CST for constant stores, MEM_REF for memory copy or
      BIT_*_EXPR for logical bitwise operation.  */
   enum tree_code rhs_code;
+  /* True if BIT_{AND,IOR,XOR}_EXPR result is inverted before storing.  */
   bool bit_not_p;
+  /* True if ops have been swapped and thus ops[1] represents
+     rhs1 of BIT_{AND,IOR,XOR}_EXPR and ops[0] represents rhs2.  */
+  bool ops_swapped_p;
   /* Operands.  For BIT_*_EXPR rhs_code both operands are used, otherwise
      just the first one.  */
   store_operand_info ops[2];
@@ -231,7 +235,8 @@ store_immediate_info::store_immediate_in
 					    const store_operand_info &op0r,
 					    const store_operand_info &op1r)
   : bitsize (bs), bitpos (bp), bitregion_start (brs), bitregion_end (bre),
-    stmt (st), order (ord), rhs_code (rhscode), bit_not_p (bitnotp)
+    stmt (st), order (ord), rhs_code (rhscode), bit_not_p (bitnotp),
+    ops_swapped_p (false)
 #if __cplusplus >= 201103L
     , ops { op0r, op1r }
 {
@@ -1186,7 +1191,10 @@ imm_store_chain_info::coalesce_immediate
 		  == info->bitpos - infof->bitpos)
 	      && operand_equal_p (info->ops[1].base_addr,
 				  infof->ops[0].base_addr, 0))
-	    std::swap (info->ops[0], info->ops[1]);
+	    {
+	      std::swap (info->ops[0], info->ops[1]);
+	      info->ops_swapped_p = true;
+	    }
 	  if ((!infof->ops[0].base_addr
 	       || compatible_load_p (merged_store, info, base_addr, 0))
 	      && (!infof->ops[1].base_addr
@@ -1410,18 +1418,21 @@ count_multiple_uses (store_immediate_inf
       stmt = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (stmt));
       /* stmt is now the BIT_*_EXPR.  */
       if (!has_single_use (gimple_assign_rhs1 (stmt)))
-	ret += 1 + info->ops[0].bit_not_p;
-      else if (info->ops[0].bit_not_p)
+	ret += 1 + info->ops[info->ops_swapped_p].bit_not_p;
+      else if (info->ops[info->ops_swapped_p].bit_not_p)
 	{
 	  gimple *stmt2 = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (stmt));
 	  if (!has_single_use (gimple_assign_rhs1 (stmt2)))
 	    ++ret;
 	}
       if (info->ops[1].base_addr == NULL_TREE)
-	return ret;
+	{
+	  gcc_checking_assert (!info->ops_swapped_p);
+	  return ret;
+	}
       if (!has_single_use (gimple_assign_rhs2 (stmt)))
-	ret += 1 + info->ops[1].bit_not_p;
-      else if (info->ops[1].bit_not_p)
+	ret += 1 + info->ops[1 - info->ops_swapped_p].bit_not_p;
+      else if (info->ops[1 - info->ops_swapped_p].bit_not_p)
 	{
 	  gimple *stmt2 = SSA_NAME_DEF_STMT (gimple_assign_rhs2 (stmt));
 	  if (!has_single_use (gimple_assign_rhs1 (stmt2)))
--- gcc/testsuite/gcc.dg/pr82929.c.jj	2017-11-10 08:10:14.399799845 +0100
+++ gcc/testsuite/gcc.dg/pr82929.c	2017-11-10 08:18:24.131904561 +0100
@@ -0,0 +1,18 @@
+/* PR tree-optimization/82929 */
+/* { dg-do compile { target store_merge } } */
+/* { dg-options "-O2 -fdump-tree-store-merging" } */
+
+void
+foo (short *p, short *q, short *r)
+{
+  short a = q[0];
+  short b = q[1];
+  short c = ~a;
+  short d = r[0];
+  short e = r[1];
+  short f = ~b;
+  p[0] = c & d;
+  p[1] = e & f;
+}
+
+/* { dg-final { scan-tree-dump-times "Merging successful" 1 "store-merging" } } */
--- gcc/testsuite/g++.dg/opt/pr82929.C.jj	2017-11-10 08:17:35.843479732 +0100
+++ gcc/testsuite/g++.dg/opt/pr82929.C	2017-11-10 08:16:16.000000000 +0100
@@ -0,0 +1,30 @@
+// PR tree-optimization/82929
+// { dg-do compile }
+// { dg-options "-O2" }
+
+template <int _Nw> struct A {
+  long _M_w[_Nw];
+  void m_fn1(A p1) {
+    for (int a = 0;; a++)
+      _M_w[a] &= p1._M_w[a];
+  }
+  void m_fn2() {
+    for (int b = 0; b < _Nw; b++)
+      _M_w[b] = ~_M_w[b];
+  }
+};
+template <int _Nb> struct C : A<_Nb / (8 * 8)> {
+  void operator&=(C p1) { this->m_fn1(p1); }
+  C m_fn3() {
+    this->m_fn2();
+    return *this;
+  }
+  C operator~() { return C(*this).m_fn3(); }
+};
+struct B {
+  C<192> Value;
+};
+void fn1(C<192> &p1) {
+  B c;
+  p1 &= ~c.Value;
+}

	Jakub



More information about the Gcc-patches mailing list