Fix up store_merging_13.c FAIL

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?

Just managed to reproduce the profiledbootstrap issue, so working on that
next.

2017-11-09  Jakub Jelinek  <jakub@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.


	Jakub

On Thu, 9 Nov 2017, Jakub Jelinek wrote:

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

Sorry about that. If this canonicalization causes too much trouble, we 
could revert it (it isn't important to me). Although it looks like you 
managed to extend the store merging code nicely instead :-)

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  <jakub@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
> 
>