[ping][vect-patterns][RFC] Refactor widening patterns to allow internal_fn's

[ping][vect-patterns][RFC] Refactor widening patterns to allow internal_fn's

[Joel Hutton]

Ping. Is there still interest in refactoring vect-patterns to internal_fn's? 

> -----Original Message-----
> From: Joel Hutton
> Sent: 07 June 2021 14:30
> To: gcc-patches@gcc.gnu.org
> Cc: Richard Biener <rguenther@suse.de>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Subject: [vect-patterns][RFC] Refactor widening patterns to allow
> internal_fn's
> 
> Hi all,
> 
> This refactor allows widening patterns (such as widen_plus/widen_minus) to
> be represented as either internal_fns or tree_codes. The widening patterns
> were originally added as tree codes with the expectation that they would be
> refactored later.
> 
> [vect-patterns] Refactor as internal_fn's
> 
> Refactor vect-patterns to allow patterns to be internal_fns starting with
> widening_plus/minus patterns.
> 
> 
> gcc/ChangeLog:
> 
>         * gimple-match.h (class code_helper): Move code_helper class to more
> visible header.
>         * internal-fn.h (internal_fn_name): Add internal_fn range check.
>         * optabs-tree.h (supportable_convert_operation): Change function
> prototypes to use code_helper.
>         * tree-vect-patterns.c (vect_recog_widen_op_pattern): Refactor to use
> code_helper.
>         * tree-vect-stmts.c (vect_gen_widened_results_half): Refactor to use
> code_helper, build internal_fns.
>         (vect_create_vectorized_promotion_stmts): Refactor to use
> code_helper.
>         (vectorizable_conversion): Refactor to use code_helper.
>         (supportable_widening_operation): Refactor to use code_helper.
>         (supportable_narrowing_operation): Refactor to use code_helper.
>         * tree-vectorizer.h (supportable_widening_operation): Refactor to use
> code_helper.
>         (supportable_narrowing_operation): Refactor to use code_helper.
>         * tree.h (class code_helper): Refactor to use code_helper.

Re: [ping][vect-patterns][RFC] Refactor widening patterns to allow internal_fn's

[Richard Biener]

On Tue, 17 Aug 2021, Joel Hutton wrote:

> Ping. Is there still interest in refactoring vect-patterns to internal_fn's? 

Yes, sorry ...

+      internal_fn fn = as_internal_fn ((combined_fn) wide_code_or_ifn);

maybe add an overload to as_internal_fn.

+      pattern_stmt = gimple_build_call_internal (fn, 2,
+                                       oprnd[0],
+                                       oprnd[1]);

some whitespace problems here?  It looks like it fits on less lines.

+  if (ch.is_tree_code ())
+  {
+    new_stmt = gimple_build_assign (vec_dest, ch, vec_oprnd0, 
vec_oprnd1);
+    new_temp = make_ssa_name (vec_dest, new_stmt);
+    gimple_assign_set_lhs (new_stmt, new_temp);
+  }
+  else
+  {
+      new_stmt = gimple_build_call_internal (as_internal_fn 
((combined_fn) ch),
+                                            2, vec_oprnd0, vec_oprnd1);
+      new_temp = make_ssa_name (vec_dest, new_stmt);
+      gimple_call_set_lhs (new_stmt, new_temp);
+  }

only the new_stmt = needs to be conditional if you use
gimple_set_lhs ().

+  if (is_gimple_assign (stmt))
+  {
+    code_or_ifn = gimple_assign_rhs_code (stmt);
+  }
+  else
+    code_or_ifn = (combined_fn) gimple_call_combined_fn (stmt);

is the cast really necessary?

+  if (is_gimple_assign (stmt))
+  {
+    if (!CONVERT_EXPR_CODE_P (code_or_ifn)
+       && code_or_ifn != FIX_TRUNC_EXPR
+       && code_or_ifn != FLOAT_EXPR
+       && code_or_ifn != WIDEN_PLUS_EXPR
+       && code_or_ifn != WIDEN_MINUS_EXPR
+       && code_or_ifn != WIDEN_MULT_EXPR
+       && code_or_ifn != WIDEN_LSHIFT_EXPR)
+      return false;
+  }
+

no constraints for the IFN case?  The check should be
code_or_ifn.is_tree_code () for clarity.

+      if (supportable_convert_operation (code_or_ifn, vectype_out, 
vectype_in,
+         (tree_code*) &code_or_ifn1))

that looks fragile (TBAA), better pass &code1 and if successful
update code_or_ifn1 from it?

I wonder if we should add as_tree_code () / as_fn_code ()
that work safely when the code isn't a tree or fn code like

   enum tree_code as_tree_code () const { return is_tree_code () ? 
(tree_code)*this : MAX_TREE_CODES; }

and similar for as_fn_code (with CFN_LAST).  That would make

+  if (code_or_ifn.is_tree_code ())
+  {
+    switch ((tree_code) code_or_ifn)
+      {

nicer to be just

  switch (code_or_ifn.as_tree_code ())
   {
...
   }
  switch (code_or_ifn.as_fn_code ())
   {
...
   }

that said, the patch doesn't include any actual new IFNs for the
widening stuff, correct?  Still thanks for the work sofar.

Thanks,
Richard.

> > -----Original Message-----
> > From: Joel Hutton
> > Sent: 07 June 2021 14:30
> > To: gcc-patches@gcc.gnu.org
> > Cc: Richard Biener <rguenther@suse.de>; Richard Sandiford
> > <Richard.Sandiford@arm.com>
> > Subject: [vect-patterns][RFC] Refactor widening patterns to allow
> > internal_fn's
> > 
> > Hi all,
> > 
> > This refactor allows widening patterns (such as widen_plus/widen_minus) to
> > be represented as either internal_fns or tree_codes. The widening patterns
> > were originally added as tree codes with the expectation that they would be
> > refactored later.
> > 
> > [vect-patterns] Refactor as internal_fn's
> > 
> > Refactor vect-patterns to allow patterns to be internal_fns starting with
> > widening_plus/minus patterns.
> > 
> > 
> > gcc/ChangeLog:
> > 
> >         * gimple-match.h (class code_helper): Move code_helper class to more
> > visible header.
> >         * internal-fn.h (internal_fn_name): Add internal_fn range check.
> >         * optabs-tree.h (supportable_convert_operation): Change function
> > prototypes to use code_helper.
> >         * tree-vect-patterns.c (vect_recog_widen_op_pattern): Refactor to use
> > code_helper.
> >         * tree-vect-stmts.c (vect_gen_widened_results_half): Refactor to use
> > code_helper, build internal_fns.
> >         (vect_create_vectorized_promotion_stmts): Refactor to use
> > code_helper.
> >         (vectorizable_conversion): Refactor to use code_helper.
> >         (supportable_widening_operation): Refactor to use code_helper.
> >         (supportable_narrowing_operation): Refactor to use code_helper.
> >         * tree-vectorizer.h (supportable_widening_operation): Refactor to use
> > code_helper.
> >         (supportable_narrowing_operation): Refactor to use code_helper.
> >         * tree.h (class code_helper): Refactor to use code_helper.
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)