Hi Richard,

on 2019/7/10 下午7:50, Richard Biener wrote:
> On Mon, 8 Jul 2019, Kewen.Lin wrote:
> 
> 
> +      tree rhs = gimple_assign_rhs1 (oe1def);
> +      tree vec = TREE_OPERAND (rhs, 0);
> +      tree vec_type = TREE_TYPE (vec);
> +
> +      if (TREE_CODE (vec) != SSA_NAME || !VECTOR_TYPE_P (vec_type))
> +       continue;
> ...
> +      /* Ignore it if target machine can't support this VECTOR type.  */
> +      if (!VECTOR_MODE_P (TYPE_MODE (vec_type)))
> +       continue;
> 
> put the check below next to the one above, it's cheaper than the
> poly-int stuff that currently preceeds it.
> 
> +      v_info_ptr *info_ptr = v_info_map.get (vec);
> +      if (info_ptr)
> +       {
> 
> there is get_or_insert which enables you to mostly merge the two cases
> with a preceeding
> 
>   if (!existed)
>     v_info_ptr = new v_info;
> 
> also eliding the extra hash-lookup the put operation otherwise requires.
> 
> +  for (hash_map<tree, v_info_ptr>::iterator it = v_info_map.begin ();
> +       it != v_info_map.end (); ++it)
> +    {
> +      tree cand_vec = (*it).first;
> +      v_info_ptr cand_info = (*it).second;
> +      unsigned int num_elems = VECTOR_CST_NELTS (cand_vec).to_constant 
> ();
> 
> please add a quick out like
> 
>       if (cand_info->length () != num_elems)
>         continue;
> 
> since to have no holes and no overlap you need exactly that many.
> 
> I think you can avoid the somewhat ugly mode_to_total counter array.
> Since you have sorted valid_vecs after modes simply do
> 
>   for (unsigned i = 0; i < valid_vecs.length () - 1; ++i)
>     {
>       tree tvec = valid_vecs[i];
>       enum machine_mode mode = TYPE_MODE (TREE_TYPE (tvec));
> 
> (please don't use unsigned int for mode!)
> 
>       /* Skip modes with only a single candidate.  */
>       if (TYPE_MODE (TREE_TYPE (valid_vecs[i+1])) != mode)
>         continue;
> 
>       do
>         {
> ...
>         }
>       while (...)
> 
> Richard.
> 

Thanks a lot for your comments, I've updated the patch accordingly (also
including Segher's comments on test cases).

Bootstrapped and regression test passed on powerpc64le-unknown-linux-gnu
and x86_64-linux-gnu.

Is it ok for trunk? 

Thanks,
Kewen

-----------

gcc/ChangeLog

2019-07-11  Kewen Lin  <linkw@gcc.gnu.org>

	PR tree-optimization/88497
	* tree-ssa-reassoc.c (reassociate_bb): Swap the positions of
	GIMPLE_BINARY_RHS check and gimple_visited_p check, call new
	function undistribute_bitref_for_vector.
	(undistribute_bitref_for_vector): New function.
	(cleanup_vinfo_map): Likewise.
	(sort_by_mach_mode): Likewise.

gcc/testsuite/ChangeLog

2019-07-11  Kewen Lin  <linkw@gcc.gnu.org>

	* gcc.dg/tree-ssa/pr88497-1.c: New test.
	* gcc.dg/tree-ssa/pr88497-2.c: Likewise.
	* gcc.dg/tree-ssa/pr88497-3.c: Likewise.
	* gcc.dg/tree-ssa/pr88497-4.c: Likewise.
	* gcc.dg/tree-ssa/pr88497-5.c: Likewise.
	* gcc.dg/tree-ssa/pr88497-6.c: Likewise.
	* gcc.dg/tree-ssa/pr88497-7.c: Likewise.