public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <rguenther@suse.de>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Martin Jambor <mjambor@suse.cz>,
	Aldy Hernandez <aldyh@redhat.com>,
	 gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] ipa: Remove ipa_bits
Date: Fri, 6 Oct 2023 05:54:57 +0000 (UTC)	[thread overview]
Message-ID: <nycvar.YFH.7.77.849.2310060554500.5561@jbgna.fhfr.qr> (raw)
In-Reply-To: <ZR6rbAKYvtP+kIzu@tucnak>

On Thu, 5 Oct 2023, Jakub Jelinek wrote:

> Hi!
> 
> The following patch removes ipa_bits struct pointer/vector from ipa
> jump functions and ipa cp transformations.
> 
> The reason is because the struct uses widest_int to represent
> mask/value pair, which in the RFC patches to allow larger precisions
> for wide_int/widest_int is GC unfriendly because those types become
> non-trivially default constructible/copyable/destructible.
> One option would be to use trailing_wide_int for that instead, but
> as pointed out by Aldy, irange_storage which we already use under
> the hood for ipa_vr when type of parameter is integral or pointer
> already stores the mask/value pair because VRP now does the bit cp
> as well.
> So, this patch just uses m_vr to store both the value range and
> the bitmask.  There is still separate propagation of the
> ipcp_bits_lattice from propagation of the ipcp_vr_lattice, but
> when storing we merge the two into the same container.
> 
> I've bootstrapped/regtested a slightly older version of this
> patch on x86_64-linux and i686-linux and that version regressed
> +FAIL: gcc.dg/ipa/propalign-3.c scan-ipa-dump-not cp "align:"
> +FAIL: gcc.dg/ipa/propalign-3.c scan-tree-dump optimized "fail_the_test"
> +FAIL: gcc.dg/ipa/propbits-1.c scan-ipa-dump cp "Adjusting mask for param 0 to 0x7"
> +FAIL: gcc.dg/ipa/propbits-2.c scan-ipa-dump cp "Adjusting mask for param 0 to 0xf"
> The last 2 were solely about the earlier patch not actually copying
> the if (dump_file) dumping of message that we set some mask for some
> parameter (since then added in the @@ -5985,6 +5741,77 @@ hunk).
> The first testcase is a test for -fno-ipa-bit-cp disabling bit cp
> for alignments.  For integral types I'm afraid it is a lost case
> when -fno-ipa-bit-cp -fipa-vrp is on when value ranges track bit cp
> as well, but for pointer alignments I've added
>   && opt_for_fn (cs->caller->decl, flag_ipa_bit_cp)
> and
>   && opt_for_fn (node->decl, flag_ipa_bit_cp)
> guards such that even just -fno-ipa-bit-cp disables it (alternatively
> we could just add -fno-ipa-vrp to propalign-3.c dg-options).
> 
> Ok for trunk if this passes another bootstrap/regtest?

OK.

Richard.

> Or defer until it is really needed (when the wide_int/widest_int
> changes are about to be committed)?
> 
> 2023-10-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* ipa-prop.h (ipa_bits): Remove.
> 	(struct ipa_jump_func): Remove bits member.
> 	(struct ipcp_transformation): Remove bits member, adjust
> 	ctor and dtor.
> 	(ipa_get_ipa_bits_for_value): Remove.
> 	* ipa-prop.cc (struct ipa_bit_ggc_hash_traits): Remove.
> 	(ipa_bits_hash_table): Remove.
> 	(ipa_print_node_jump_functions_for_edge): Don't print bits.
> 	(ipa_get_ipa_bits_for_value): Remove.
> 	(ipa_set_jfunc_bits): Remove.
> 	(ipa_compute_jump_functions_for_edge): For pointers query
> 	pointer alignment before ipa_set_jfunc_vr and update_bitmask
> 	in there.  For integral types, just rely on bitmask already
> 	being handled in value ranges.
> 	(ipa_check_create_edge_args): Don't create ipa_bits_hash_table.
> 	(ipcp_transformation_initialize): Neither here.
> 	(ipcp_transformation_t::duplicate): Don't copy bits vector.
> 	(ipa_write_jump_function): Don't stream bits here.
> 	(ipa_read_jump_function): Neither here.
> 	(useful_ipcp_transformation_info_p): Don't test bits vec.
> 	(write_ipcp_transformation_info): Don't stream bits here.
> 	(read_ipcp_transformation_info): Neither here.
> 	(ipcp_get_parm_bits): Get mask and value from m_vr rather
> 	than bits.
> 	(ipcp_update_bits): Remove.
> 	(ipcp_update_vr): For pointers, set_ptr_info_alignment from
> 	bitmask stored in value range.
> 	(ipcp_transform_function): Don't test bits vector, don't call
> 	ipcp_update_bits.
> 	* ipa-cp.cc (propagate_bits_across_jump_function): Don't use
> 	jfunc->bits, instead get mask and value from jfunc->m_vr.
> 	(ipcp_store_bits_results): Remove.
> 	(ipcp_store_vr_results): Incorporate parts of
> 	ipcp_store_bits_results here, merge the bitmasks with value
> 	range if both are supplied.
> 	(ipcp_driver): Don't call ipcp_store_bits_results.
> 	* ipa-sra.cc (zap_useless_ipcp_results): Remove *ts->bits
> 	clearing.
> 
> --- gcc/ipa-prop.h.jj	2023-10-05 11:32:40.172739988 +0200
> +++ gcc/ipa-prop.h	2023-10-05 11:36:45.405378086 +0200
> @@ -292,18 +292,6 @@ public:
>    array_slice<const ipa_argagg_value> m_elts;
>  };
>  
> -/* Information about zero/non-zero bits.  */
> -class GTY(()) ipa_bits
> -{
> -public:
> -  /* The propagated value.  */
> -  widest_int value;
> -  /* Mask corresponding to the value.
> -     Similar to ccp_lattice_t, if xth bit of mask is 0,
> -     implies xth bit of value is constant.  */
> -  widest_int mask;
> -};
> -
>  /* Info about value ranges.  */
>  
>  class GTY(()) ipa_vr
> @@ -342,11 +330,6 @@ struct GTY (()) ipa_jump_func
>       and its description.  */
>    struct ipa_agg_jump_function agg;
>  
> -  /* Information about zero/non-zero bits.  The pointed to structure is shared
> -     betweed different jump functions.  Use ipa_set_jfunc_bits to set this
> -     field.  */
> -  class ipa_bits *bits;
> -
>    /* Information about value range, containing valid data only when vr_known is
>       true.  The pointed to structure is shared betweed different jump
>       functions.  Use ipa_set_jfunc_vr to set this field.  */
> @@ -940,15 +923,13 @@ struct GTY(()) ipcp_transformation
>  {
>    /* Default constructor.  */
>    ipcp_transformation ()
> -    : m_agg_values (nullptr), bits (nullptr), m_vr (nullptr),
> -    m_uid_to_idx (nullptr)
> +    : m_agg_values (nullptr), m_vr (nullptr), m_uid_to_idx (nullptr)
>    { }
>  
>    /* Default destructor.  */
>    ~ipcp_transformation ()
>    {
>      vec_free (m_agg_values);
> -    vec_free (bits);
>      vec_free (m_vr);
>    }
>  
> @@ -968,8 +949,6 @@ struct GTY(()) ipcp_transformation
>  
>    /* Known aggregate values.  */
>    vec<ipa_argagg_value, va_gc>  *m_agg_values;
> -  /* Known bits information.  */
> -  vec<ipa_bits *, va_gc> *bits;
>    /* Value range information.  */
>    vec<ipa_vr, va_gc> *m_vr;
>    /* If there are many parameters, this is a vector sorted by their DECL_UIDs
> @@ -1172,8 +1151,6 @@ tree ipa_get_indirect_edge_target (struc
>  struct cgraph_edge *ipa_make_edge_direct_to_target (struct cgraph_edge *, tree,
>  						    bool speculative = false);
>  tree ipa_impossible_devirt_target (struct cgraph_edge *, tree);
> -ipa_bits *ipa_get_ipa_bits_for_value (const widest_int &value,
> -				      const widest_int &mask);
>  
>  
>  /* Functions related to both.  */
> --- gcc/ipa-prop.cc.jj	2023-10-05 11:32:40.154740234 +0200
> +++ gcc/ipa-prop.cc	2023-10-05 14:06:13.742885839 +0200
> @@ -66,49 +66,6 @@ function_summary <ipcp_transformation *>
>  /* Edge summary for IPA-CP edge information.  */
>  ipa_edge_args_sum_t *ipa_edge_args_sum;
>  
> -/* Traits for a hash table for reusing already existing ipa_bits. */
> -
> -struct ipa_bit_ggc_hash_traits : public ggc_cache_remove <ipa_bits *>
> -{
> -  typedef ipa_bits *value_type;
> -  typedef ipa_bits *compare_type;
> -  static hashval_t
> -  hash (const ipa_bits *p)
> -  {
> -    hashval_t t = (hashval_t) p->value.to_shwi ();
> -    return iterative_hash_host_wide_int (p->mask.to_shwi (), t);
> -  }
> -  static bool
> -  equal (const ipa_bits *a, const ipa_bits *b)
> -    {
> -      return a->value == b->value && a->mask == b->mask;
> -    }
> -  static const bool empty_zero_p = true;
> -  static void
> -  mark_empty (ipa_bits *&p)
> -    {
> -      p = NULL;
> -    }
> -  static bool
> -  is_empty (const ipa_bits *p)
> -    {
> -      return p == NULL;
> -    }
> -  static bool
> -  is_deleted (const ipa_bits *p)
> -    {
> -      return p == reinterpret_cast<const ipa_bits *> (1);
> -    }
> -  static void
> -  mark_deleted (ipa_bits *&p)
> -    {
> -      p = reinterpret_cast<ipa_bits *> (1);
> -    }
> -};
> -
> -/* Hash table for avoid repeated allocations of equal ipa_bits.  */
> -static GTY ((cache)) hash_table<ipa_bit_ggc_hash_traits> *ipa_bits_hash_table;
> -
>  /* Traits for a hash table for reusing ranges.  */
>  
>  struct ipa_vr_ggc_hash_traits : public ggc_cache_remove <ipa_vr *>
> @@ -528,17 +485,6 @@ ipa_print_node_jump_functions_for_edge (
>  	  ctx->dump (dump_file);
>  	}
>  
> -      if (jump_func->bits)
> -	{
> -	  fprintf (f, "         value: ");
> -	  print_hex (jump_func->bits->value, f);
> -	  fprintf (f, ", mask: ");
> -	  print_hex (jump_func->bits->mask, f);
> -	  fprintf (f, "\n");
> -	}
> -      else
> -	fprintf (f, "         Unknown bits\n");
> -
>        if (jump_func->m_vr)
>  	{
>  	  jump_func->m_vr->dump (f);
> @@ -2267,39 +2213,6 @@ ipa_get_callee_param_type (struct cgraph
>    return NULL;
>  }
>  
> -/* Return ipa_bits with VALUE and MASK values, which can be either a newly
> -   allocated structure or a previously existing one shared with other jump
> -   functions and/or transformation summaries.  */
> -
> -ipa_bits *
> -ipa_get_ipa_bits_for_value (const widest_int &value, const widest_int &mask)
> -{
> -  ipa_bits tmp;
> -  tmp.value = value;
> -  tmp.mask = mask;
> -
> -  ipa_bits **slot = ipa_bits_hash_table->find_slot (&tmp, INSERT);
> -  if (*slot)
> -    return *slot;
> -
> -  ipa_bits *res = ggc_alloc<ipa_bits> ();
> -  res->value = value;
> -  res->mask = mask;
> -  *slot = res;
> -
> -  return res;
> -}
> -
> -/* Assign to JF a pointer to ipa_bits structure with VALUE and MASK.  Use hash
> -   table in order to avoid creating multiple same ipa_bits structures.  */
> -
> -static void
> -ipa_set_jfunc_bits (ipa_jump_func *jf, const widest_int &value,
> -		    const widest_int &mask)
> -{
> -  jf->bits = ipa_get_ipa_bits_for_value (value, mask);
> -}
> -
>  /* Return a pointer to an ipa_vr just like TMP, but either find it in
>     ipa_vr_hash_table or allocate it in GC memory.  */
>  
> @@ -2393,10 +2306,31 @@ ipa_compute_jump_functions_for_edge (str
>  	    addr_nonzero = true;
>  
>  	  if (addr_nonzero)
> +	    vr.set_nonzero (TREE_TYPE (arg));
> +
> +	  unsigned HOST_WIDE_INT bitpos;
> +	  unsigned align, prec = TYPE_PRECISION (TREE_TYPE (arg));
> +
> +	  get_pointer_alignment_1 (arg, &align, &bitpos);
> +
> +	  if (align > BITS_PER_UNIT
> +	      && opt_for_fn (cs->caller->decl, flag_ipa_bit_cp))
>  	    {
> -	      vr.set_nonzero (TREE_TYPE (arg));
> +	      wide_int mask
> +		= wi::bit_and_not (wi::mask (prec, false, prec),
> +				   wide_int::from (align / BITS_PER_UNIT - 1,
> +						   prec, UNSIGNED));
> +	      wide_int value = wide_int::from (bitpos / BITS_PER_UNIT, prec,
> +					       UNSIGNED);
> +	      irange_bitmask bm (value, mask);
> +	      if (!addr_nonzero)
> +		vr.set_varying (TREE_TYPE (arg));
> +	      irange &r = as_a <irange> (vr);
> +	      r.update_bitmask (bm);
>  	      ipa_set_jfunc_vr (jfunc, vr);
>  	    }
> +	  else if (addr_nonzero)
> +	    ipa_set_jfunc_vr (jfunc, vr);
>  	  else
>  	    gcc_assert (!jfunc->m_vr);
>  	}
> @@ -2421,30 +2355,6 @@ ipa_compute_jump_functions_for_edge (str
>  	    gcc_assert (!jfunc->m_vr);
>  	}
>  
> -      if (INTEGRAL_TYPE_P (TREE_TYPE (arg)) && !vr.undefined_p ())
> -	{
> -	  irange &r = as_a <irange> (vr);
> -	  irange_bitmask bm = r.get_bitmask ();
> -	  signop sign = TYPE_SIGN (TREE_TYPE (arg));
> -	  ipa_set_jfunc_bits (jfunc,
> -			      widest_int::from (bm.value (), sign),
> -			      widest_int::from (bm.mask (), sign));
> -	}
> -      else if (POINTER_TYPE_P (TREE_TYPE (arg)))
> -	{
> -	  unsigned HOST_WIDE_INT bitpos;
> -	  unsigned align;
> -
> -	  get_pointer_alignment_1 (arg, &align, &bitpos);
> -	  widest_int mask = wi::bit_and_not
> -	    (wi::mask<widest_int> (TYPE_PRECISION (TREE_TYPE (arg)), false),
> -	     align / BITS_PER_UNIT - 1);
> -	  widest_int value = bitpos / BITS_PER_UNIT;
> -	  ipa_set_jfunc_bits (jfunc, value, mask);
> -	}
> -      else
> -	gcc_assert (!jfunc->bits);
> -
>        if (is_gimple_ip_invariant (arg)
>  	  || (VAR_P (arg)
>  	      && is_global_var (arg)
> @@ -4398,8 +4308,6 @@ ipa_check_create_edge_args (void)
>      ipa_edge_args_sum
>        = (new (ggc_alloc_no_dtor<ipa_edge_args_sum_t> ())
>  	 ipa_edge_args_sum_t (symtab, true));
> -  if (!ipa_bits_hash_table)
> -    ipa_bits_hash_table = hash_table<ipa_bit_ggc_hash_traits>::create_ggc (37);
>    if (!ipa_vr_hash_table)
>      ipa_vr_hash_table = hash_table<ipa_vr_ggc_hash_traits>::create_ggc (37);
>  }
> @@ -4432,8 +4340,6 @@ ipa_free_all_node_params (void)
>  void
>  ipcp_transformation_initialize (void)
>  {
> -  if (!ipa_bits_hash_table)
> -    ipa_bits_hash_table = hash_table<ipa_bit_ggc_hash_traits>::create_ggc (37);
>    if (!ipa_vr_hash_table)
>      ipa_vr_hash_table = hash_table<ipa_vr_ggc_hash_traits>::create_ggc (37);
>    if (ipcp_transformation_sum == NULL)
> @@ -4636,7 +4542,6 @@ ipcp_transformation_t::duplicate(cgraph_
>    if (dst->inlined_to)
>      return;
>    dst_trans->m_agg_values = vec_safe_copy (src_trans->m_agg_values);
> -  dst_trans->bits = vec_safe_copy (src_trans->bits);
>    dst_trans->m_vr = vec_safe_copy (src_trans->m_vr);
>  }
>  
> @@ -4859,13 +4764,6 @@ ipa_write_jump_function (struct output_b
>      }
>  
>    bp = bitpack_create (ob->main_stream);
> -  bp_pack_value (&bp, !!jump_func->bits, 1);
> -  streamer_write_bitpack (&bp);
> -  if (jump_func->bits)
> -    {
> -      streamer_write_widest_int (ob, jump_func->bits->value);
> -      streamer_write_widest_int (ob, jump_func->bits->mask);
> -    }
>    if (jump_func->m_vr)
>      jump_func->m_vr->streamer_write (ob);
>    else
> @@ -4992,18 +4890,6 @@ ipa_read_jump_function (class lto_input_
>          jump_func->agg.items->quick_push (item);
>      }
>  
> -  struct bitpack_d bp = streamer_read_bitpack (ib);
> -  bool bits_known = bp_unpack_value (&bp, 1);
> -  if (bits_known)
> -    {
> -      widest_int value = streamer_read_widest_int (ib);
> -      widest_int mask = streamer_read_widest_int (ib);
> -      if (prevails)
> -	ipa_set_jfunc_bits (jump_func, value, mask);
> -    }
> -  else
> -    jump_func->bits = NULL;
> -
>    ipa_vr vr;
>    vr.streamer_read (ib, data_in);
>    if (vr.known_p ())
> @@ -5387,7 +5273,6 @@ useful_ipcp_transformation_info_p (ipcp_
>    if (!ts)
>      return false;
>    if (!vec_safe_is_empty (ts->m_agg_values)
> -      || !vec_safe_is_empty (ts->bits)
>        || !vec_safe_is_empty (ts->m_vr))
>      return true;
>    return false;
> @@ -5420,19 +5305,6 @@ write_ipcp_transformation_info (output_b
>    streamer_write_uhwi (ob, vec_safe_length (ts->m_vr));
>    for (const ipa_vr &parm_vr : ts->m_vr)
>      parm_vr.streamer_write (ob);
> -
> -  streamer_write_uhwi (ob, vec_safe_length (ts->bits));
> -  for (const ipa_bits *bits_jfunc : ts->bits)
> -    {
> -      struct bitpack_d bp = bitpack_create (ob->main_stream);
> -      bp_pack_value (&bp, !!bits_jfunc, 1);
> -      streamer_write_bitpack (&bp);
> -      if (bits_jfunc)
> -	{
> -	  streamer_write_widest_int (ob, bits_jfunc->value);
> -	  streamer_write_widest_int (ob, bits_jfunc->mask);
> -	}
> -    }
>  }
>  
>  /* Stream in the aggregate value replacement chain for NODE from IB.  */
> @@ -5473,24 +5345,6 @@ read_ipcp_transformation_info (lto_input
>  	  parm_vr->streamer_read (ib, data_in);
>  	}
>      }
> -  count = streamer_read_uhwi (ib);
> -  if (count > 0)
> -    {
> -      vec_safe_grow_cleared (ts->bits, count, true);
> -      for (i = 0; i < count; i++)
> -	{
> -	  struct bitpack_d bp = streamer_read_bitpack (ib);
> -	  bool known = bp_unpack_value (&bp, 1);
> -	  if (known)
> -	    {
> -	      const widest_int value = streamer_read_widest_int (ib);
> -	      const widest_int mask = streamer_read_widest_int (ib);
> -	      ipa_bits *bits
> -		= ipa_get_ipa_bits_for_value (value, mask);
> -	      (*ts->bits)[i] = bits;
> -	    }
> -	}
> -    }
>  }
>  
>  /* Write all aggregate replacement for nodes in set.  */
> @@ -5796,7 +5650,9 @@ ipcp_get_parm_bits (tree parm, tree *val
>  {
>    cgraph_node *cnode = cgraph_node::get (current_function_decl);
>    ipcp_transformation *ts = ipcp_get_transformation_summary (cnode);
> -  if (!ts || vec_safe_length (ts->bits) == 0)
> +  if (!ts
> +      || vec_safe_length (ts->m_vr) == 0
> +      || !irange::supports_p (TREE_TYPE (parm)))
>      return false;
>  
>    int i = ts->get_param_index (current_function_decl, parm);
> @@ -5810,120 +5666,20 @@ ipcp_get_parm_bits (tree parm, tree *val
>  	return false;
>      }
>  
> -  vec<ipa_bits *, va_gc> &bits = *ts->bits;
> -  if (!bits[i])
> +  vec<ipa_vr, va_gc> &vr = *ts->m_vr;
> +  if (!vr[i].known_p ())
>      return false;
> -  *mask = bits[i]->mask;
> -  *value = wide_int_to_tree (TREE_TYPE (parm), bits[i]->value);
> +  Value_Range tmp;
> +  vr[i].get_vrange (tmp);
> +  if (tmp.undefined_p () || tmp.varying_p ())
> +    return false;
> +  irange &r = as_a <irange> (tmp);
> +  irange_bitmask bm = r.get_bitmask ();
> +  *mask = widest_int::from (bm.mask (), TYPE_SIGN (TREE_TYPE (parm)));
> +  *value = wide_int_to_tree (TREE_TYPE (parm), bm.value ());
>    return true;
>  }
>  
> -/* Update bits info of formal parameters of NODE as described in TS.  */
> -
> -static void
> -ipcp_update_bits (struct cgraph_node *node, ipcp_transformation *ts)
> -{
> -  if (vec_safe_is_empty (ts->bits))
> -    return;
> -  vec<ipa_bits *, va_gc> &bits = *ts->bits;
> -  unsigned count = bits.length ();
> -  if (!count)
> -    return;
> -
> -  auto_vec<int, 16> new_indices;
> -  bool need_remapping = false;
> -  clone_info *cinfo = clone_info::get (node);
> -  if (cinfo && cinfo->param_adjustments)
> -    {
> -      cinfo->param_adjustments->get_updated_indices (&new_indices);
> -      need_remapping = true;
> -    }
> -  auto_vec <tree, 16> parm_decls;
> -  push_function_arg_decls (&parm_decls, node->decl);
> -
> -  for (unsigned i = 0; i < count; ++i)
> -    {
> -      tree parm;
> -      if (need_remapping)
> -	{
> -	  if (i >= new_indices.length ())
> -	    continue;
> -	  int idx = new_indices[i];
> -	  if (idx < 0)
> -	    continue;
> -	  parm = parm_decls[idx];
> -	}
> -      else
> -	parm = parm_decls[i];
> -      gcc_checking_assert (parm);
> -
> -
> -      if (!bits[i]
> -	  || !(INTEGRAL_TYPE_P (TREE_TYPE (parm))
> -	       || POINTER_TYPE_P (TREE_TYPE (parm)))
> -	  || !is_gimple_reg (parm))
> -	continue;
> -
> -      tree ddef = ssa_default_def (DECL_STRUCT_FUNCTION (node->decl), parm);
> -      if (!ddef)
> -	continue;
> -
> -      if (dump_file)
> -	{
> -	  fprintf (dump_file, "Adjusting mask for param %u to ", i);
> -	  print_hex (bits[i]->mask, dump_file);
> -	  fprintf (dump_file, "\n");
> -	}
> -
> -      if (INTEGRAL_TYPE_P (TREE_TYPE (ddef)))
> -	{
> -	  unsigned prec = TYPE_PRECISION (TREE_TYPE (ddef));
> -	  signop sgn = TYPE_SIGN (TREE_TYPE (ddef));
> -	  wide_int mask = wide_int::from (bits[i]->mask, prec, UNSIGNED);
> -	  wide_int value = wide_int::from (bits[i]->value, prec, sgn);
> -	  set_bitmask (ddef, value, mask);
> -	}
> -      else
> -	{
> -	  unsigned tem = bits[i]->mask.to_uhwi ();
> -	  unsigned HOST_WIDE_INT bitpos = bits[i]->value.to_uhwi ();
> -	  unsigned align = tem & -tem;
> -	  unsigned misalign = bitpos & (align - 1);
> -
> -	  if (align > 1)
> -	    {
> -	      if (dump_file)
> -		fprintf (dump_file, "Adjusting align: %u, misalign: %u\n", align, misalign); 
> -
> -	      unsigned old_align, old_misalign;
> -	      struct ptr_info_def *pi = get_ptr_info (ddef);
> -	      bool old_known = get_ptr_info_alignment (pi, &old_align, &old_misalign);
> -
> -	      if (old_known
> -		  && old_align > align)
> -		{
> -		  if (dump_file)
> -		    {
> -		      fprintf (dump_file, "But alignment was already %u.\n", old_align);
> -		      if ((old_misalign & (align - 1)) != misalign)
> -			fprintf (dump_file, "old_misalign (%u) and misalign (%u) mismatch\n",
> -				 old_misalign, misalign);
> -		    }
> -		  continue;
> -		}
> -
> -	      if (old_known
> -		  && ((misalign & (old_align - 1)) != old_misalign)
> -		  && dump_file)
> -		fprintf (dump_file, "old_misalign (%u) and misalign (%u) mismatch\n",
> -			 old_misalign, misalign);
> -
> -	      set_ptr_info_alignment (pi, align, misalign); 
> -	    }
> -	}
> -    }
> -}
> -
>  /* Update value range of formal parameters of NODE as described in TS.  */
>  
>  static void
> @@ -5985,6 +5741,77 @@ ipcp_update_vr (struct cgraph_node *node
>  		  fprintf (dump_file, "]\n");
>  		}
>  	      set_range_info (ddef, tmp);
> +
> +	      if (POINTER_TYPE_P (TREE_TYPE (parm))
> +		  && opt_for_fn (node->decl, flag_ipa_bit_cp))
> +		{
> +		  irange &r = as_a<irange> (tmp);
> +		  irange_bitmask bm = r.get_bitmask ();
> +		  unsigned tem = bm.mask ().to_uhwi ();
> +		  unsigned HOST_WIDE_INT bitpos = bm.value ().to_uhwi ();
> +		  unsigned align = tem & -tem;
> +		  unsigned misalign = bitpos & (align - 1);
> +
> +		  if (align > 1)
> +		    {
> +		      if (dump_file)
> +			{
> +			  fprintf (dump_file,
> +				   "Adjusting mask for param %u to ", i);
> +			  print_hex (bm.mask (), dump_file);
> +			  fprintf (dump_file, "\n");
> +			}
> +
> +		      if (dump_file)
> +			fprintf (dump_file,
> +				 "Adjusting align: %u, misalign: %u\n",
> +				 align, misalign);
> +
> +		      unsigned old_align, old_misalign;
> +		      struct ptr_info_def *pi = get_ptr_info (ddef);
> +		      bool old_known = get_ptr_info_alignment (pi, &old_align,
> +							       &old_misalign);
> +
> +		      if (old_known && old_align > align)
> +			{
> +			  if (dump_file)
> +			    {
> +			      fprintf (dump_file,
> +				       "But alignment was already %u.\n",
> +				       old_align);
> +			      if ((old_misalign & (align - 1)) != misalign)
> +				fprintf (dump_file,
> +					 "old_misalign (%u) and misalign "
> +					 "(%u) mismatch\n",
> +					 old_misalign, misalign);
> +			    }
> +			  continue;
> +			}
> +
> +		      if (dump_file
> +			  && old_known
> +			  && ((misalign & (old_align - 1)) != old_misalign))
> +			fprintf (dump_file,
> +				 "old_misalign (%u) and misalign (%u) "
> +				 "mismatch\n",
> +				 old_misalign, misalign);
> +
> +		      set_ptr_info_alignment (pi, align, misalign);
> +		    }
> +		}
> +	      else if (dump_file && INTEGRAL_TYPE_P (TREE_TYPE (parm)))
> +		{
> +		  irange &r = as_a<irange> (tmp);
> +		  irange_bitmask bm = r.get_bitmask ();
> +		  unsigned prec = TYPE_PRECISION (TREE_TYPE (parm));
> +		  if (wi::ne_p (bm.mask (), wi::shwi (-1, prec)))
> +		    {
> +		      fprintf (dump_file,
> +			       "Adjusting mask for param %u to ", i);
> +		      print_hex (bm.mask (), dump_file);
> +		      fprintf (dump_file, "\n");
> +		    }
> +		}
>  	    }
>  	}
>      }
> @@ -6008,12 +5835,10 @@ ipcp_transform_function (struct cgraph_n
>    ipcp_transformation *ts = ipcp_get_transformation_summary (node);
>    if (!ts
>        || (vec_safe_is_empty (ts->m_agg_values)
> -	  && vec_safe_is_empty (ts->bits)
>  	  && vec_safe_is_empty (ts->m_vr)))
>      return 0;
>  
>    ts->maybe_create_parm_idx_map (cfun->decl);
> -  ipcp_update_bits (node, ts);
>    ipcp_update_vr (node, ts);
>    if (vec_safe_is_empty (ts->m_agg_values))
>        return 0;
> --- gcc/ipa-cp.cc.jj	2023-10-05 11:32:39.828744703 +0200
> +++ gcc/ipa-cp.cc	2023-10-05 11:36:45.408378045 +0200
> @@ -2749,11 +2749,22 @@ propagate_bits_across_jump_function (cgr
>  	}
>      }
>  
> -  if (jfunc->bits)
> -    return dest_lattice->meet_with (jfunc->bits->value, jfunc->bits->mask,
> -				    precision);
> -  else
> -    return dest_lattice->set_to_bottom ();
> +  Value_Range vr (parm_type);
> +  if (jfunc->m_vr)
> +    {
> +      jfunc->m_vr->get_vrange (vr);
> +      if (!vr.undefined_p () && !vr.varying_p ())
> +	{
> +	  irange &r = as_a <irange> (vr);
> +	  irange_bitmask bm = r.get_bitmask ();
> +	  widest_int mask
> +	    = widest_int::from (bm.mask (), TYPE_SIGN (parm_type));
> +	  widest_int value
> +	    = widest_int::from (bm.value (), TYPE_SIGN (parm_type));
> +	  return dest_lattice->meet_with (value, mask, precision);
> +	}
> +    }
> +  return dest_lattice->set_to_bottom ();
>  }
>  
>  /* Propagate value range across jump function JFUNC that is associated with
> @@ -6521,89 +6532,8 @@ ipcp_decision_stage (class ipa_topo_info
>      }
>  }
>  
> -/* Look up all the bits information that we have discovered and copy it over
> -   to the transformation summary.  */
> -
> -static void
> -ipcp_store_bits_results (void)
> -{
> -  cgraph_node *node;
> -
> -  FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
> -    {
> -      ipa_node_params *info = ipa_node_params_sum->get (node);
> -      bool dumped_sth = false;
> -      bool found_useful_result = false;
> -
> -      if (!opt_for_fn (node->decl, flag_ipa_bit_cp) || !info)
> -	{
> -	  if (dump_file)
> -	    fprintf (dump_file, "Not considering %s for ipa bitwise propagation "
> -				"; -fipa-bit-cp: disabled.\n",
> -				node->dump_name ());
> -	  continue;
> -	}
> -
> -      if (info->ipcp_orig_node)
> -	info = ipa_node_params_sum->get (info->ipcp_orig_node);
> -      if (!info->lattices)
> -	/* Newly expanded artificial thunks do not have lattices.  */
> -	continue;
> -
> -      unsigned count = ipa_get_param_count (info);
> -      for (unsigned i = 0; i < count; i++)
> -	{
> -	  ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
> -	  if (plats->bits_lattice.constant_p ())
> -	    {
> -	      found_useful_result = true;
> -	      break;
> -	    }
> -	}
> -
> -      if (!found_useful_result)
> -	continue;
> -
> -      ipcp_transformation_initialize ();
> -      ipcp_transformation *ts = ipcp_transformation_sum->get_create (node);
> -      vec_safe_reserve_exact (ts->bits, count);
> -
> -      for (unsigned i = 0; i < count; i++)
> -	{
> -	  ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
> -	  ipa_bits *jfbits;
> -
> -	  if (plats->bits_lattice.constant_p ())
> -	    {
> -	      jfbits
> -		= ipa_get_ipa_bits_for_value (plats->bits_lattice.get_value (),
> -					      plats->bits_lattice.get_mask ());
> -	      if (!dbg_cnt (ipa_cp_bits))
> -		jfbits = NULL;
> -	    }
> -	  else
> -	    jfbits = NULL;
> -
> -	  ts->bits->quick_push (jfbits);
> -	  if (!dump_file || !jfbits)
> -	    continue;
> -	  if (!dumped_sth)
> -	    {
> -	      fprintf (dump_file, "Propagated bits info for function %s:\n",
> -		       node->dump_name ());
> -	      dumped_sth = true;
> -	    }
> -	  fprintf (dump_file, " param %i: value = ", i);
> -	  print_hex (jfbits->value, dump_file);
> -	  fprintf (dump_file, ", mask = ");
> -	  print_hex (jfbits->mask, dump_file);
> -	  fprintf (dump_file, "\n");
> -	}
> -    }
> -}
> -
> -/* Look up all VR information that we have discovered and copy it over
> -   to the transformation summary.  */
> +/* Look up all VR and bits information that we have discovered and copy it
> +   over to the transformation summary.  */
>  
>  static void
>  ipcp_store_vr_results (void)
> @@ -6613,7 +6543,10 @@ ipcp_store_vr_results (void)
>    FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
>      {
>        ipa_node_params *info = ipa_node_params_sum->get (node);
> +      bool dumped_sth = false;
>        bool found_useful_result = false;
> +      bool do_vr = true;
> +      bool do_bits = true;
>  
>        if (!info || !opt_for_fn (node->decl, flag_ipa_vrp))
>  	{
> @@ -6621,8 +6554,18 @@ ipcp_store_vr_results (void)
>  	    fprintf (dump_file, "Not considering %s for VR discovery "
>  		     "and propagate; -fipa-ipa-vrp: disabled.\n",
>  		     node->dump_name ());
> -	  continue;
> +	  do_vr = false;
> +	}
> +      if (!info || !opt_for_fn (node->decl, flag_ipa_bit_cp))
> +	{
> +	  if (dump_file)
> +	    fprintf (dump_file, "Not considering %s for ipa bitwise "
> +				"propagation ; -fipa-bit-cp: disabled.\n",
> +				node->dump_name ());
> +	  do_bits = false;
>  	}
> +      if (!do_bits && !do_vr)
> +	continue;
>  
>        if (info->ipcp_orig_node)
>  	info = ipa_node_params_sum->get (info->ipcp_orig_node);
> @@ -6634,12 +6577,18 @@ ipcp_store_vr_results (void)
>        for (unsigned i = 0; i < count; i++)
>  	{
>  	  ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
> -	  if (!plats->m_value_range.bottom_p ()
> +	  if (do_vr
> +	      && !plats->m_value_range.bottom_p ()
>  	      && !plats->m_value_range.top_p ())
>  	    {
>  	      found_useful_result = true;
>  	      break;
>  	    }
> +	  if (do_bits && plats->bits_lattice.constant_p ())
> +	    {
> +	      found_useful_result = true;
> +	      break;
> +	    }
>  	}
>        if (!found_useful_result)
>  	continue;
> @@ -6651,12 +6600,53 @@ ipcp_store_vr_results (void)
>        for (unsigned i = 0; i < count; i++)
>  	{
>  	  ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
> +	  ipcp_bits_lattice *bits = NULL;
> +
> +	  if (do_bits
> +	      && plats->bits_lattice.constant_p ()
> +	      && dbg_cnt (ipa_cp_bits))
> +	    bits = &plats->bits_lattice;
>  
> -	  if (!plats->m_value_range.bottom_p ()
> +	  if (do_vr
> +	      && !plats->m_value_range.bottom_p ()
>  	      && !plats->m_value_range.top_p ()
>  	      && dbg_cnt (ipa_cp_vr))
>  	    {
> -	      ipa_vr vr (plats->m_value_range.m_vr);
> +	      if (bits)
> +		{
> +		  Value_Range tmp = plats->m_value_range.m_vr;
> +		  tree type = ipa_get_type (info, i);
> +		  irange &r = as_a<irange> (tmp);
> +		  irange_bitmask bm (wide_int::from (bits->get_value (),
> +						     TYPE_PRECISION (type),
> +						     TYPE_SIGN (type)),
> +				     wide_int::from (bits->get_mask (),
> +						     TYPE_PRECISION (type),
> +						     TYPE_SIGN (type)));
> +		  r.update_bitmask (bm);
> +		  ipa_vr vr (tmp);
> +		  ts->m_vr->quick_push (vr);
> +		}
> +	      else
> +		{
> +		  ipa_vr vr (plats->m_value_range.m_vr);
> +		  ts->m_vr->quick_push (vr);
> +		}
> +	    }
> +	  else if (bits)
> +	    {
> +	      tree type = ipa_get_type (info, i);
> +	      Value_Range tmp;
> +	      tmp.set_varying (type);
> +	      irange &r = as_a<irange> (tmp);
> +	      irange_bitmask bm (wide_int::from (bits->get_value (),
> +						 TYPE_PRECISION (type),
> +						 TYPE_SIGN (type)),
> +				 wide_int::from (bits->get_mask (),
> +						 TYPE_PRECISION (type),
> +						 TYPE_SIGN (type)));
> +	      r.update_bitmask (bm);
> +	      ipa_vr vr (tmp);
>  	      ts->m_vr->quick_push (vr);
>  	    }
>  	  else
> @@ -6664,6 +6654,21 @@ ipcp_store_vr_results (void)
>  	      ipa_vr vr;
>  	      ts->m_vr->quick_push (vr);
>  	    }
> +
> +	  if (!dump_file || !bits)
> +	    continue;
> +
> +	  if (!dumped_sth)
> +	    {
> +	      fprintf (dump_file, "Propagated bits info for function %s:\n",
> +		       node->dump_name ());
> +	      dumped_sth = true;
> +	    }
> +	  fprintf (dump_file, " param %i: value = ", i);
> +	  print_hex (bits->get_value (), dump_file);
> +	  fprintf (dump_file, ", mask = ");
> +	  print_hex (bits->get_mask (), dump_file);
> +	  fprintf (dump_file, "\n");
>  	}
>      }
>  }
> @@ -6696,9 +6701,7 @@ ipcp_driver (void)
>    ipcp_propagate_stage (&topo);
>    /* Decide what constant propagation and cloning should be performed.  */
>    ipcp_decision_stage (&topo);
> -  /* Store results of bits propagation.  */
> -  ipcp_store_bits_results ();
> -  /* Store results of value range propagation.  */
> +  /* Store results of value range and bits propagation.  */
>    ipcp_store_vr_results ();
>  
>    /* Free all IPCP structures.  */
> --- gcc/ipa-sra.cc.jj	2023-10-05 11:32:40.233739151 +0200
> +++ gcc/ipa-sra.cc	2023-10-05 11:36:45.408378045 +0200
> @@ -4134,22 +4134,8 @@ zap_useless_ipcp_results (const isra_fun
>    else if (removed_item)
>      ts->m_agg_values->truncate (dst_index);
>  
> -  bool useful_bits = false;
> -  unsigned count = vec_safe_length (ts->bits);
> -  for (unsigned i = 0; i < count; i++)
> -    if ((*ts->bits)[i])
> -    {
> -      const isra_param_desc *desc = &(*ifs->m_parameters)[i];
> -      if (desc->locally_unused)
> -	(*ts->bits)[i] = NULL;
> -      else
> -	useful_bits = true;
> -    }
> -  if (!useful_bits)
> -    ts->bits = NULL;
> -
>    bool useful_vr = false;
> -  count = vec_safe_length (ts->m_vr);
> +  unsigned count = vec_safe_length (ts->m_vr);
>    for (unsigned i = 0; i < count; i++)
>      if ((*ts->m_vr)[i].known_p ())
>        {
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

      parent reply	other threads:[~2023-10-06  5:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-05 12:26 Jakub Jelinek
2023-10-05 14:06 ` Aldy Hernandez
2023-10-05 14:42 ` Jan Hubicka
2023-10-05 14:59   ` Jakub Jelinek
2023-10-06  5:54 ` Richard Biener [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=nycvar.YFH.7.77.849.2310060554500.5561@jbgna.fhfr.qr \
    --to=rguenther@suse.de \
    --cc=aldyh@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=mjambor@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).