On Fri, Apr 19, 2013 at 7:15 PM, James Greenhalgh
<james.greenha...@arm.com> wrote:
> Hi Richard,
>
> Thanks for your feedback. This does feel like a much nicer solution now.
>
>> 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.
>
> This seems sensible - hopefully something like the attached will be
> closer to an acceptable implementation. The aarch64 portion certainly
> looks much cleaner now.
>
>> 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?
>
> I've gone for TARGET_GIMPLE_FOLD_BUILTIN (to fit with the
> non-target-specific gimple_fold_builtin).
>
> I've also updated the documentation for TARGET_FOLD_BUILTIN to
> make it clear that the tree returned must be valid in GENERIC
> and GIMPLE.
>
> Thanks again for your help.
The middle-end changes are ok with moving
@@ -1145,6 +1145,13 @@ gimple_fold_call (gimple_stmt_iterator *gsi,
bool inplace)
}
}
+ /* Call target-specific, GIMPLE only fold routines. */
+ callee = gimple_call_fndecl (stmt);
+ if (!changed
+ && callee
+ && DECL_BUILT_IN_CLASS (callee) == BUILT_IN_MD)
+ changed |= targetm.gimple_fold_builtin (gsi);
+
return changed;
}
this inside the already existing
callee = gimple_call_fndecl (stmt);
if (callee && DECL_BUILT_IN (callee))
{
tree result = gimple_fold_builtin (stmt);
if (result)
{
if (!update_call_from_tree (gsi, result))
gimplify_and_update_call_from_tree (gsi, result);
changed = true;
}
check - right here, as
else if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_MD)
changed |= targetm.gimple_fold_builtin (gsi);
also,
+/* Fold a target-specific builtin to a valid GIMPLE tree. */
+DEFHOOK
+(gimple_fold_builtin,
+ "Like @samp{TARGET_FOLD_BUILTIN}, but without the restriction that\n\
+the result tree be valid for GENERIC. Fold a call to a machine specific\n\
+built-in function that was set up by @samp{TARGET_INIT_BUILTINS}.\n\
+@var{gsi} points to the gimple statement holding the function call.\n\
+Returns true if any change was made to the GIMPLE stream.",
please drop the first sentence of the description.
Thanks,
Richard.
> James
> Graduate Engineer
> ARM.
>
> ---
> gcc/
>
> 2013-04-19 James Greenhalgh <james.greenha...@arm.com>
>
> * coretypes.h (gimple_stmt_iterator_d): Forward declare.
> (gimple_stmt_iterator): New typedef.
> * gimple.h (gimple_stmt_iterator): Rename to...
> (gimple_stmt_iterator_d): ... This.
> * doc/tm.texi.in (TARGET_FOLD_BUILTIN): Detail restriction that
> trees be valid for GIMPLE and GENERIC.
> (TARGET_GIMPLE_FOLD_BUILTIN): New.
> * gimple-fold.c (gimple_fold_call): Call target hook
> gimple_fold_builtin.
> * hooks.c (hook_bool_gsiptr_false): New.
> * hooks.h (hook_bool_gsiptr_false): New.
> * target.def (fold_stmt): New.
> * doc/tm.texi: Regenerate.
>
> 2013-04-19 James Greenhalgh <james.greenha...@arm.com>
>
> * config/aarch64/aarch64-builtins.c
> (aarch64_fold_builtin): Move folds to gimple-specific tree codes
> to aarch64_gimple_fold_builtin.
> (aarch64_gimple_fold_builtin): New.
> * config/aarch64/aarch64-protos.h (aarch64_gimple_fold_builtin): New.
> * config/aarch64/aarch64.c (TARGET_GIMPLE_FOLD_BUILTIN): Define.
