On Fri, Apr 19, 2013 at 11:39 AM, James Greenhalgh
<[email protected]> wrote:
> Hi,
>
>> I still think getting rid of TARGET_FOLD_BUILTIN and replacing it with
>> TARGET_FOLD_STMT that only operates on GIMPLE is the way to go.
>
> Would that look something like the attached? (The patch isn't
> polished, but hacks enough together to function).
>
> In these patches we add TARGET_FOLD_STMT and an implimentation in
> aarch64_fold_stmt. TARGET_FOLD_STMT takes a gimple_stmt_iterator
> and returns whether any change was made to the gimple stream.
>
> Then TARGET_FOLD_STMT is allowed to fold to anything which is
> valid GIMPLE, and TARGET_FOLD_BUILTIN is required to fold to
> something in the intersection of GIMPLE and GENERIC.
>
> I don't especially like passing a gimple_stmt_iterator as it means
> exposing all sorts of extra gunk to the backend. Without it, the
> interface is not as flexible. This is not neccessarily a problem, but
> giving the backend the full gimple_stmt_iterator seems to me to be
> more useful.
>
> If this was what you were asking for I can clean the patch up
> and give it proper testing. If not, I would appreciate a prod
> in the correct direction.
Yes, it looks basically ok. I'd probably restrict it to folding target builtins
though - similar to what TARGET_FOLD_BUILTIN did. Exactly to not
expose all stmts to the backends. That is, move the target hook call
to gimple_fold_call, after the inplace check (and remove the inplace
argument of the target hook), and call it only for DECL_BUILT_IN_CLASS
BUILT_IN_MD.
Not sure if TARGET_FOLD_STMT is then still an appropriate name ...
TARGET_FOLD_BUILTIN is already taken unfortunately. Maybe
TARGET_FOLD_BUILTIN_MD_CALL?
+ fndecl = gimple_call_fndecl (stmt);
+ if (fndecl
+ && TREE_CODE (fndecl) == FUNCTION_DECL
redundant check
+ && DECL_BUILT_IN (fndecl)
+ && !gimple_call_va_arg_pack_p (stmt)
+ && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD)
the last check is enough, the previous two are not necessary.
+ if (avoid_folding_inline_builtin (fndecl))
+ break;
I don't think this is necessary.
And for simple cases such as...
+ BUILTIN_VALL (UNOP, reduc_splus_, 10)
+ new_tree = fold_build1 (REDUC_PLUS_EXPR,
+ type,
+ args[0]);
...
+ if (new_tree && ignore)
+ STRIP_NOPS (new_tree);
+
+ if (new_tree)
+ {
+ gimplify_and_update_call_from_tree (gsi, new_tree);
+ changed = true;
+ }
please build the resulting assingment yourself. Like
gimple new_stmt = gimple_build_assign_with_ops (REDUC_PLUS_EXPR,
gimple_call_lhs (call), args[0], NULL_TREE);
gsi_replace (gsi, new_stmt, true);
instead of building a tree and then using gimplify_and_update_call_from_tree.
Thanks,
Richard.
Thanks,
Richard.
> Thanks,
>
> James Greenhalgh
> Graduate Engineer
> ARM
>
> ---
> gcc/
>
> 2013-04-19 James Greenhalgh <[email protected]>
>
> * coretypes.h (gimple_stmt_iterator_d): Forward declare.
> (gimple_stmt_iterator): New typedef.
> * doc/tm.texi.in (TARGET_FOLD_STMT): New.
> * gimple-fold.c (fold_stmt_1): Call target hook fold_stmt.
> * gimple.h (gimple_stmt_iterator): Rename to...
> (gimple_stmt_iterator_d): ... This.
> * hooks.c (hook_bool_gsiptr_bool_false): New.
> * target.def (fold_stmt): New.
> * doc/tm.texi: Regenerate.
>
> 2013-04-19 James Greenhalgh <[email protected]>
>
> * config/aarch64/aarch64-builtins.c
> (aarch64_fold_builtin): Move folds to gimple-specific tree codes
> to aarch64_fold_stmt.
> (aarch64_fold_stmt): New.
> * config/aarch64/aarch64-protos.h (aarch64_fold_stmt): New.
> * config/aarch64/aarch64.c (TARGET_FOLD_STMT): Define.
