This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C++11][4.9] Add missing REDUC_PLUS_EXPR case to potential_constant_expression_1.


On Fri, Apr 19, 2013 at 11:39 AM, James Greenhalgh
<james.greenhalgh@arm.com> 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  <james.greenhalgh@arm.com>
>
>         * 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  <james.greenhalgh@arm.com>
>
>         * 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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]