On Thu, 11 Jul 2019, Kewen.Lin wrote:
> 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?
This is OK.
Thanks,
Richard.
> Thanks,
> Kewen
>
> -----------
>
> gcc/ChangeLog
>
> 2019-07-11 Kewen Lin <li...@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 <li...@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.
>
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
