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 <ja...@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