This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Mon, 8 Jul 2019, Kewen.Lin wrote:

> Hi Richard,
> 
> Thanks a lot for your review and reply, I've updated the patch accordingly.
> 
> Main changes compared to previous one:
>   - Consider SVE (poly_int) on bit_field_ref size/offset.
>   - Filter valid candidates with sbitmap first.
>   - Support multiple modes (TODO 1).
>   - Test cases: add SSE2 support for 1..5, update run result check for 1,
>     add two more cases (5,6)to verify multiple mode support (x86).
> 
> Bootstrapped and regression test passed on powerpc64le-unknown-linux-gnu
> and x86_64-linux-gnu.
> 
> Could you help to review it again?  Thanks in advance!

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

> Kewen
> 
> ----
> 
> gcc/ChangeLog
> 
> 2019-07-08  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-08  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)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]