On Tue, 31 May 2022, Joel Hutton wrote:

> > Can you post an updated patch (after the .cc renaming, and code_helper
> > now already moved to tree.h).
> > 
> > Thanks,
> > Richard.
> 
> Patches attached. They already incorporated the .cc rename, now rebased to be 
> after the change to tree.h

@@ -1412,8 +1412,7 @@ vect_recog_widen_op_pattern (vec_info *vinfo,
                       2, oprnd, half_type, unprom, vectype);

   tree var = vect_recog_temp_ssa_var (itype, NULL);
-  gimple *pattern_stmt = gimple_build_assign (var, wide_code,
-                                             oprnd[0], oprnd[1]);
+  gimple *pattern_stmt = gimple_build (var, wide_code, oprnd[0], 
oprnd[1]);


you should be able to do without the new gimple_build overload
by using

   gimple_seq stmts = NULL;
   gimple_build (&stmts, wide_code, itype, oprnd[0], oprnd[1]);
   gimple *pattern_stmt = gimple_seq_last_stmt (stmts);

because 'gimple_build' is an existing API.


-  if (TREE_CODE (gimple_assign_lhs (stmt)) != SSA_NAME)
+  if (gimple_get_lhs (stmt) == NULL_TREE ||
+      TREE_CODE(gimple_get_lhs (stmt)) != SSA_NAME)
     return false;

|| go to the next line, space after TREE_CODE

+  bool widen_arith = false;
+  gimple_match_op res_op;
+  if (!gimple_extract_op (stmt, &res_op))
+    return false;
+  code = res_op.code;
+  op_type = res_op.num_ops;
+
+  if (is_gimple_assign (stmt))
+  {
+      widen_arith = (code == WIDEN_PLUS_EXPR
+                    || code == WIDEN_MINUS_EXPR
+                    || code == WIDEN_MULT_EXPR
+                    || code == WIDEN_LSHIFT_EXPR);
+ }
+  else
+      widen_arith = gimple_call_flags (stmt) & ECF_WIDEN;

there seem to be formatting issues.  Also shouldn't you check
if (res_op.code.is_tree_code ()) instead if is_gimple_assign?
I also don't like the ECF_WIDEN "trick", just do as with the
tree codes and explicitely enumerate widening ifns here.

gimple_extract_op is a bit heavy-weight as well, so maybe
instead simply do

  if (is_gimple_assign (stmt))
    {
      code = gimple_assign_rhs_code (stmt);
...
    }
  else if (gimple_call_internal_p (stmt))
    {
      code = gimple_call_internal_fn (stmt);
...
    }
  else
    return false;

+  code_helper c1=MAX_TREE_CODES, c2=MAX_TREE_CODES;

spaces before/after '='

@@ -12061,13 +12105,16 @@ supportable_widening_operation (vec_info *vinfo,
   if (BYTES_BIG_ENDIAN && c1 != VEC_WIDEN_MULT_EVEN_EXPR)
     std::swap (c1, c2);

+
   if (code == FIX_TRUNC_EXPR)
     {

unnecessary whitespace change.

diff --git a/gcc/tree.h b/gcc/tree.h
index 
f84958933d51144bb6ce7cc41eca5f7f06814550..e51e34c051d9b91d1c02a4b2fefdb2b15606a36f
 
100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -92,6 +92,10 @@ public:
   bool is_fn_code () const { return rep < 0; }
   bool is_internal_fn () const;
   bool is_builtin_fn () const;
+  enum tree_code as_tree_code () const { return is_tree_code () ?
+    (tree_code)* this : MAX_TREE_CODES; }
+  combined_fn as_fn_code () const { return is_fn_code () ? (combined_fn) 
*this
+    : CFN_LAST;}

hmm, the other as_* functions we have are not member functions.
Also this semantically differs from the tree_code () conversion
operator (that one was supposed to be "cheap").  The existing
as_internal_fn for example is documented as being valid only if
the code is actually an internal fn.  I see you are introducing
the new function as convenience to get a "safe" not-a-X value,
so maybe they should be called safe_as_tree_code () instead?


   int get_rep () const { return rep; }
   bool operator== (const code_helper &other) { return rep == other.rep; }
   bool operator!= (const code_helper &other) { return rep != other.rep; }
@@ -6657,6 +6661,54 @@ extern unsigned fndecl_dealloc_argno (tree);
    if nonnull, set the second argument to the referenced enclosing
    object or pointer.  Otherwise return null.  */
 extern tree get_attr_nonstring_decl (tree, tree * = NULL);
+/* Helper to transparently allow tree codes and builtin function codes
+   exist in one storage entity.  */
+class code_helper
+{

duplicate add of code_helper.

Sorry to raise these issues so late.

Richard.

Reply via email to