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: Random cleanups [2/4]: canonicalize ctor values


On Thu, Mar 31, 2011 at 5:57 PM, Michael Matz <matz@suse.de> wrote:
> On Thu, 31 Mar 2011, Richard Guenther wrote:
>
>> > canonicalize_constructor_val may be doing useful things on ADDR_EXPR
>> > too, but you don't call it in that case because you only added your
>> > code in the default case. ?You only reach it when the ADDR_EXPR is
>> > wrapped in a cast.
>> >
>> > Any reason why you didn't add it (possibly wrapped in a loop) _before_
>> > the switch statement?
>
> Nope, probably I didn't want to pay the cost each time, but you're right.
>
>> Indeed. ?This comment needs adjustment, too:
>>
>> ? ? ? /* We never have the chance to fixup types in global initializers
>> ? ? ? ? ?during gimplification. ?Do so here. ?*/
>
> Done in new patch.
>
>> as we now sort-of do this. ?In fact it looks like all existing
>> canonicalize_constructor_val calls are subsumed by the lowering, so,
>> eventually remove those and move this function to a local place next to
>> its sole caller.
>
> Yeah. ?I couldn't immediately convince myself that all simplifications
> will also be done on local vars (for which record_reference won't run), so
> I'll leave this for later (I'm for instance doubtful about the NULL return
> for when the current unit "can't" refer to a symbol). ?FWIW I'm currently
> testing with these asserts which should tell me:
>
> -----------------------------------------------------
> @@ -152,7 +151,8 @@ get_symbol_constant_value (tree sym)
> ? ? ? tree val = DECL_INITIAL (sym);
> ? ? ? if (val)
> ? ? ? ?{
> - ? ? ? ? val = canonicalize_constructor_val (val);
> + ? ? ? ? tree newval = canonicalize_constructor_val (val);
> + ? ? ? ? gcc_assert (newval == val);
> ? ? ? ? ?if (val && is_gimple_min_invariant (val))
> ? ? ? ? ? ?return val;
> ? ? ? ? ?else
> @@ -3164,7 +3164,11 @@ fold_ctor_reference (tree type, tree cto
> ? /* We found the field with exact match. ?*/
> ? if (useless_type_conversion_p (type, TREE_TYPE (ctor))
> ? ? ? && !offset)
> - ? ?return canonicalize_constructor_val (ctor);
> + ? ?{
> + ? ? ?tree newctor = canonicalize_constructor_val (ctor);
> + ? ? ?gcc_assert (newctor == ctor);
> + ? ? ?return newctor;
> + ? ?}
>
> ? /* We are at the end of walk, see if we can view convert the
> ? ? ?result. ?*/
> @@ -3174,6 +3178,7 @@ fold_ctor_reference (tree type, tree cto
> ? ? ? ? ? ? ? ? ? ? ? ? ?TYPE_SIZE (TREE_TYPE (ctor)), 0))
> ? ? {
> ? ? ? ret = canonicalize_constructor_val (ctor);
> + ? ? ?gcc_assert (ret == ctor);
> ? ? ? ret = fold_unary (VIEW_CONVERT_EXPR, type, ret);
> ? ? ? if (ret)
> ? ? ? ?STRIP_NOPS (ret);
> -----------------------------------------------------
>
> In the meanwhile, is the below version okay?

If it bootstraps & tests ok then yes.  The java parts look obvious.

Thanks,
Richard.

>
> Ciao,
> Michael.
> --
> ? ? ? ?* cgraphbuild.c (record_reference): Canonicalize constructor
> ? ? ? ?values.
> ? ? ? ?* gimple-fold.c (canonicalize_constructor_val): Accept being called
> ? ? ? ?without function context.
>
> java/
> ? ? ? ?* jcf-parse.c (java_emit_static_constructor): Clear cfun and
> ? ? ? ?current_function_decl.
>
> Index: cgraphbuild.c
> ===================================================================
> --- cgraphbuild.c.orig ?2011-03-29 17:08:15.000000000 +0200
> +++ cgraphbuild.c ? ? ? 2011-03-31 17:43:29.000000000 +0200
> @@ -53,6 +53,12 @@ record_reference (tree *tp, int *walk_su
> ? tree decl;
> ? struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;
>
> + ?t = canonicalize_constructor_val (t);
> + ?if (!t)
> + ? ?t = *tp;
> + ?else if (t != *tp)
> + ? ?*tp = t;
> +
> ? switch (TREE_CODE (t))
> ? ? {
> ? ? case VAR_DECL:
> Index: gimple-fold.c
> ===================================================================
> --- gimple-fold.c.orig ?2011-03-29 17:08:15.000000000 +0200
> +++ gimple-fold.c ? ? ? 2011-03-31 17:42:45.000000000 +0200
> @@ -106,7 +106,7 @@ can_refer_decl_in_current_unit_p (tree d
> ? return true;
> ?}
>
> -/* CVAL is value taken from DECL_INITIAL of variable. ?Try to transorm it into
> +/* CVAL is value taken from DECL_INITIAL of variable. ?Try to transform it into
> ? ?acceptable form for is_gimple_min_invariant. ? */
>
> ?tree
> @@ -131,10 +131,9 @@ canonicalize_constructor_val (tree cval)
> ? ? ? ? ? ? ?|| TREE_CODE (base) == FUNCTION_DECL)
> ? ? ? ? ?&& !can_refer_decl_in_current_unit_p (base))
> ? ? ? ?return NULL_TREE;
> - ? ? ?if (base && TREE_CODE (base) == VAR_DECL)
> + ? ? ?if (cfun && base && TREE_CODE (base) == VAR_DECL)
> ? ? ? ?add_referenced_var (base);
> - ? ? ?/* We never have the chance to fixup types in global initializers
> - ? ? ? ? during gimplification. ?Do so here. ?*/
> + ? ? ?/* Fixup types in global initializers. ?*/
> ? ? ? if (TREE_TYPE (TREE_TYPE (cval)) != TREE_TYPE (TREE_OPERAND (cval, 0)))
> ? ? ? ?cval = build_fold_addr_expr (TREE_OPERAND (cval, 0));
> ? ? }
> Index: java/jcf-parse.c
> ===================================================================
> --- java/jcf-parse.c.orig ? ? ? 2011-03-29 17:08:15.000000000 +0200
> +++ java/jcf-parse.c ? ?2011-03-29 17:12:05.000000000 +0200
> @@ -1725,6 +1725,8 @@ java_emit_static_constructor (void)
> ? ? ? DECL_STATIC_CONSTRUCTOR (decl) = 1;
> ? ? ? java_genericize (decl);
> ? ? ? cgraph_finalize_function (decl, false);
> + ? ? ?current_function_decl = NULL;
> + ? ? ?set_cfun (NULL);
> ? ? }
> ?}
>


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