Re: [PATCH]middle-end: Recursively check is_trivially_copyable_or_pair in vec.h

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Oct 05, 2023 at 02:01:40PM +0000, Tamar Christina wrote:
> gcc/ChangeLog:
> 
> 	* tree-if-conv.cc (INCLUDE_ALGORITHM): Remove.
> 	(typedef struct ifcvt_arg_entry): New.
> 	(cmp_arg_entry): New.
> 	(gen_phi_arg_condition, gen_phi_nest_statement,
> 	predicate_scalar_phi): Use them.

> -  /* Compute phi_arg_map.  */
> +  /* Compute phi_arg_map, determine the list of unique PHI args and the indices
> +     where they are in the PHI node.  The indices will be used to determine
> +     the conditions to apply and their complexity.  */
> +  auto_vec<tree> unique_args (num_args);
>    for (i = 0; i < num_args; i++)
>      {
>        tree arg;
>  
>        arg = gimple_phi_arg_def (phi, i);
>        if (!phi_arg_map.get (arg))
> -	args.quick_push (arg);
> +	unique_args.quick_push (arg);
>        phi_arg_map.get_or_insert (arg).safe_push (i);
>      }

I meant instead of using another vector (unique_args) just do
	args.quick_push ({ arg, 0, 0, NULL });
above (to avoid needing another allocation etc.).

> -  /* Determine element with max number of occurrences and complexity.  Looking at only
> -     number of occurrences as a measure for complexity isn't enough as all usages can
> -     be unique but the comparisons to reach the PHI node differ per branch.  */
> -  typedef std::pair <tree, std::pair <unsigned, unsigned>> ArgEntry;
> -  auto_vec<ArgEntry> argsKV;
> -  for (i = 0; i < args.length (); i++)
> +  /* Determine element with max number of occurrences and complexity.  Looking
> +     at only number of occurrences as a measure for complexity isn't enough as
> +     all usages can be unique but the comparisons to reach the PHI node differ
> +     per branch.  */
> +  for (auto arg : unique_args)

And then
  for (auto &entry : args)
here with entry.arg instead of arg and

>      {
>        unsigned int len = 0;
> -      for (int index : phi_arg_map.get (args[i]))
> +      vec<int> *indices = phi_arg_map.get (arg);
> +      for (int index : *indices)
>  	{
>  	  edge e = gimple_phi_arg_edge (phi, index);
>  	  len += get_bb_num_predicate_stmts (e->src);
>  	}
>  
> -      unsigned occur = phi_arg_map.get (args[i])->length ();
> +      unsigned occur = indices->length ();
>        if (dump_file && (dump_flags & TDF_DETAILS))
>  	fprintf (dump_file, "Ranking %d as len=%d, idx=%d\n", i, len, occur);
> -      argsKV.safe_push ({ args[i], { len, occur }});
> +      args.safe_push ({ arg, len, occur, indices });

either
      entry.num_compares = len;
      entry.occur = occur;
      entry.indices = indices;
here or just using entry.{num_occurrences,occur,indices} directly
instead of the extra automatic vars.

>      }
>  
> +  unique_args.release ();

Plus drop this.

Though, if Richi or Jeff think this is ok as is, I won't stand against it.

	Jakub