> > 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.
Done. The gimple_build overload was at the request of Richard Sandiford, I assume removing it is ok with you Richard S? >From Richard Sandiford: > For example, I think we should hide this inside a new: > > gimple_build (var, wide_code, oprnd[0], oprnd[1]); > > that works directly on code_helper, similarly to the new code_helper > gimple_build interfaces. > > > - 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 > Done. > + 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. > Done. I've set widen_arith to False for the first patch as the second patch introduces the widening ifns. > 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; The patch was originally written as above, it was changed to use gimple_extract_op at the request of Richard Sandiford. I prefer gimple_extract_op as it's more compact, but I don't have strong feelings. If the Richards can agree on either version I'm happy. >From Richard Sandiford: > > + if (is_gimple_assign (stmt)) > > + { > > + code_or_ifn = gimple_assign_rhs_code (stmt); } else > > + code_or_ifn = gimple_call_combined_fn (stmt); > > It might be possible to use gimple_extract_op here (only recently added). > This would also provide the number of operands directly, instead of > needing "op_type". It would also provide an array of operands. > > + code_helper c1=MAX_TREE_CODES, c2=MAX_TREE_CODES; > > spaces before/after '=' > Done. > @@ -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. > Fixed. > diff --git a/gcc/tree.h b/gcc/tree.h > index > f84958933d51144bb6ce7cc41eca5f7f06814550..e51e34c051d9b91d1c02a4b2 > fefdb2b15606a36f > 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? > SGTM. Done > > 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. Fixed. Tests are being re-run. Ok, with changes?
0001-Refactor-to-allow-internal_fn-s.patch
Description: 0001-Refactor-to-allow-internal_fn-s.patch
0002-Refactor-widen_plus-as-internal_fn.patch
Description: 0002-Refactor-widen_plus-as-internal_fn.patch
0003-Remove-widen_plus-minus_expr-tree-codes.patch
Description: 0003-Remove-widen_plus-minus_expr-tree-codes.patch