Re: [PATCH V5] PR88497 - Extend reassoc for vector bit_field_ref

[Richard Biener <rguenther@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 3423 bytes --]

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

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)