On Thu, 9 Nov 2017, Jakub Jelinek wrote:
> Hi!
>
> The gcc.dg/store_merging_13.c testcase worked when I was bootstrapping the
> patch that introduced it, but doesn't anymore, because match.pd had
> a ~X ^ Y -> ~(X ^ Y) transformation, the code was handling only ~ on
> the loads (== arguments of &/|/^) but not on the result of the bitwise
> binary operation. The following patch handles inversion of the result
> too, after all even ~X & ~Y is ~(X | Y) and ~X | ~Y is ~(X & Y).
> In addition to this, I've added just in case some earlier passes wouldn't
> do good enough job a check that we don't recognize
> _1 = load; _2 = ~1; _3 = ~1; store = _3; and similar (or arbitrarily more
> chained BIT_NOT_EXPRs), while we'd emit a working code, the has_single_use
> accounting wouldn't be prepared for more than one BIT_NOT_EXPR on one
> operand.
>
> Ok for trunk if it passes bootstrap/regtest?
Nice.
Ok.
Thanks,
Richard.
> Just managed to reproduce the profiledbootstrap issue, so working on that
> next.
>
> 2017-11-09 Jakub Jelinek <ja...@redhat.com>
>
> * gimple-ssa-store-merging.c (struct store_immediate_info): Add
> bit_not_p field.
> (store_immediate_info::store_immediate_info): Add bitnotp argument,
> set bit_not_p to it.
> (imm_store_chain_info::coalesce_immediate_stores): Break group
> if bit_not_p is different.
> (count_multiple_uses, split_group,
> imm_store_chain_info::output_merged_store): Handle info->bit_not_p.
> (handled_load): Avoid multiple chained BIT_NOT_EXPRs.
> (pass_store_merging::process_store): Handle BIT_{AND,IOR,XOR}_EXPR
> result inverted using BIT_NOT_EXPR, compute bit_not_p, pass it
> to store_immediate_info ctor.
>
> --- gcc/gimple-ssa-store-merging.c.jj 2017-11-09 12:40:27.000000000 +0100
> +++ gcc/gimple-ssa-store-merging.c 2017-11-09 14:00:46.269673305 +0100
> @@ -209,12 +209,13 @@ 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;
> + bool bit_not_p;
> /* Operands. For BIT_*_EXPR rhs_code both operands are used, otherwise
> just the first one. */
> store_operand_info ops[2];
> store_immediate_info (unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT,
> unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT,
> - gimple *, unsigned int, enum tree_code,
> + gimple *, unsigned int, enum tree_code, bool,
> const store_operand_info &,
> const store_operand_info &);
> };
> @@ -226,10 +227,11 @@ store_immediate_info::store_immediate_in
> gimple *st,
> unsigned int ord,
> enum tree_code rhscode,
> + bool bitnotp,
> 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)
> + stmt (st), order (ord), rhs_code (rhscode), bit_not_p (bitnotp)
> #if __cplusplus >= 201103L
> , ops { op0r, op1r }
> {
> @@ -1169,7 +1171,8 @@ imm_store_chain_info::coalesce_immediate
> Merge it into the current store group. There can be gaps in between
> the stores, but there can't be gaps in between bitregions. */
> else if (info->bitregion_start <= merged_store->bitregion_end
> - && info->rhs_code == merged_store->stores[0]->rhs_code)
> + && info->rhs_code == merged_store->stores[0]->rhs_code
> + && info->bit_not_p == merged_store->stores[0]->bit_not_p)
> {
> store_immediate_info *infof = merged_store->stores[0];
>
> @@ -1386,6 +1389,17 @@ count_multiple_uses (store_immediate_inf
> case BIT_AND_EXPR:
> case BIT_IOR_EXPR:
> case BIT_XOR_EXPR:
> + if (info->bit_not_p)
> + {
> + if (!has_single_use (gimple_assign_rhs1 (stmt)))
> + ret = 1; /* Fall through below to return
> + the BIT_NOT_EXPR stmt and then
> + BIT_{AND,IOR,XOR}_EXPR and anything it
> + uses. */
> + else
> + /* stmt is after this the BIT_NOT_EXPR. */
> + stmt = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (stmt));
> + }
> if (!has_single_use (gimple_assign_rhs1 (stmt)))
> {
> ret += 1 + info->ops[0].bit_not_p;
> @@ -1479,6 +1493,8 @@ split_group (merged_store_group *group,
> case BIT_AND_EXPR:
> case BIT_IOR_EXPR:
> case BIT_XOR_EXPR:
> + if (info->bit_not_p)
> + total_orig[0]++; /* The orig BIT_NOT_EXPR stmt. */
> total_orig[0]++; /* The orig BIT_*_EXPR stmt. */
> break;
> default:
> @@ -1649,6 +1665,8 @@ split_group (merged_store_group *group,
> case BIT_AND_EXPR:
> case BIT_IOR_EXPR:
> case BIT_XOR_EXPR:
> + if (info->bit_not_p)
> + total_new[0] += ret; /* The new BIT_NOT_EXPR stmt. */
> total_new[0] += ret; /* The new BIT_*_EXPR stmt. */
> break;
> default:
> @@ -1918,6 +1936,17 @@ imm_store_chain_info::output_merged_stor
> else
> gimple_seq_add_stmt_without_update (&seq, stmt);
> src = gimple_assign_lhs (stmt);
> + if (split_store->orig_stores[0]->bit_not_p)
> + {
> + stmt = gimple_build_assign (make_ssa_name (int_type),
> + BIT_NOT_EXPR, src);
> + gimple_set_location (stmt, bit_loc);
> + if (load_addr[1] == NULL_TREE && gsi_bb (load_gsi[0]))
> + gimple_seq_add_stmt_without_update (&load_seq[0], stmt);
> + else
> + gimple_seq_add_stmt_without_update (&seq, stmt);
> + src = gimple_assign_lhs (stmt);
> + }
> break;
> default:
> src = ops[0];
> @@ -2232,6 +2261,11 @@ handled_load (gimple *stmt, store_operan
> && handled_load (SSA_NAME_DEF_STMT (rhs1), op, bitsize, bitpos,
> bitregion_start, bitregion_end))
> {
> + /* Don't allow _1 = load; _2 = ~1; _3 = ~_2; which should have
> + been optimized earlier, but if allowed here, would confuse the
> + multiple uses counting. */
> + if (op->bit_not_p)
> + return false;
> op->bit_not_p = !op->bit_not_p;
> return true;
> }
> @@ -2283,6 +2317,7 @@ pass_store_merging::process_store (gimpl
> || ((bitsize > MAX_BITSIZE_MODE_ANY_INT)
> && (TREE_CODE (rhs) != INTEGER_CST)));
> enum tree_code rhs_code = ERROR_MARK;
> + bool bit_not_p = false;
> store_operand_info ops[2];
> if (invalid)
> ;
> @@ -2301,7 +2336,17 @@ pass_store_merging::process_store (gimpl
> else if (handled_load (def_stmt, &ops[0], bitsize, bitpos,
> bitregion_start, bitregion_end))
> rhs_code = MEM_REF;
> - else
> + else if (gimple_assign_rhs_code (def_stmt) == BIT_NOT_EXPR)
> + {
> + tree rhs1 = gimple_assign_rhs1 (def_stmt);
> + if (TREE_CODE (rhs1) == SSA_NAME
> + && is_gimple_assign (SSA_NAME_DEF_STMT (rhs1)))
> + {
> + bit_not_p = true;
> + def_stmt = SSA_NAME_DEF_STMT (rhs1);
> + }
> + }
> + if (rhs_code == ERROR_MARK && !invalid)
> switch ((rhs_code = gimple_assign_rhs_code (def_stmt)))
> {
> case BIT_AND_EXPR:
> @@ -2355,7 +2400,7 @@ pass_store_merging::process_store (gimpl
> unsigned int ord = (*chain_info)->m_store_info.length ();
> info = new store_immediate_info (bitsize, bitpos, bitregion_start,
> bitregion_end, stmt, ord, rhs_code,
> - ops[0], ops[1]);
> + bit_not_p, ops[0], ops[1]);
> if (dump_file && (dump_flags & TDF_DETAILS))
> {
> fprintf (dump_file, "Recording immediate store from stmt:\n");
> @@ -2383,7 +2428,7 @@ pass_store_merging::process_store (gimpl
> = new imm_store_chain_info (m_stores_head, base_addr);
> info = new store_immediate_info (bitsize, bitpos, bitregion_start,
> bitregion_end, stmt, 0, rhs_code,
> - ops[0], ops[1]);
> + bit_not_p, ops[0], ops[1]);
> new_chain->m_store_info.safe_push (info);
> m_stores.put (base_addr, new_chain);
> if (dump_file && (dump_flags & TDF_DETAILS))
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
