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: update address taken: don't drop clobbers


On Sat, Jul 12, 2014 at 8:15 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Thu, 10 Jul 2014, Richard Biener wrote:
>
>>> --- gcc/tree-into-ssa.c (revision 212109)
>>> +++ gcc/tree-into-ssa.c (working copy)
>>> @@ -1831,26 +1831,38 @@ maybe_register_def (def_operand_p def_p,
>>>  {
>>>    tree def = DEF_FROM_PTR (def_p);
>>>    tree sym = DECL_P (def) ? def : SSA_NAME_VAR (def);
>>>
>>>    /* If DEF is a naked symbol that needs renaming, create a new
>>>       name for it.  */
>>>    if (marked_for_renaming (sym))
>>>      {
>>>        if (DECL_P (def))
>>>         {
>>> -         tree tracked_var;
>>> -
>>> -         def = make_ssa_name (def, stmt);
>>> +         if (gimple_clobber_p (stmt) && is_gimple_reg (sym))
>>
>>
>> sym should always be a gimple reg here (it's marked for renaming).
>>
>>> +           {
>>> +             /* Replace clobber stmts with a default def.  Create a new
>>> +                variable so we don't later think we must coalesce, which
>>> would
>>> +                fail with some ada abnormal PHIs.  Still, we try to keep
>>> a
>>> +                similar name so error messages make sense.  */
>>> +             unlink_stmt_vdef (stmt);
>>
>>
>> I think that's redundant with gsi_replace (note that using gsi_replace
>> looks dangerous here as it calls update_stmt during SSA rewrite...
>> that might open a can of worms).
>
>
> IIRC it was failing without unlink_stmt_vdef (maybe that was in a different
> version of the patch not using gsi_replace, but I don't think so). I was
> hoping that a clobber had little enough effects that update_stmt was
> unlikely to break anything. Anyway it doesn't matter if I use your
> suggestion below.

The important part is that it not ICE (of course) and that it doesn't
trigger useless SSA renaming of .MEM - gsi_replace does
update_stmt which does the unlink_stmt_vdef for you if the .MEM
is no longer necessary.

Richard.

>
>>> +             gsi_replace (&gsi, gimple_build_nop (), true);
>>> +             tree id = DECL_NAME (sym);
>>> +             const char* name = id ? IDENTIFIER_POINTER (id) : 0;
>>> +             tree newvar = create_tmp_var (TREE_TYPE (sym), name);
>>> +             def = get_or_create_ssa_default_def (cfun, newvar);
>>
>>
>> So - can't you simply do
>>
>>    gimple_assign_set_rhs_from_tree (&gsi,
>> get_or_create_dda_default_def (cfun, sym));
>>
>> ?  Thus replace x = CLOBBER; with x_3 = x_2(D);
>>
>>> +           }
>>> +         else
>>
>>
>> and of course still rewrite the DEF then.  IMHO the copy-propagation
>> you do is premature optimization.
>
>
> I'll try that. I was trying to remain as close as possible to what you wrote
> in rewrite_stmt:
>
>         if (gimple_clobber_p (stmt)
>             && is_gimple_reg (var))
>           {
>             /* If we rewrite a DECL into SSA form then drop its
>                clobber stmts and replace uses with a new default def.  */
>             gcc_checking_assert (TREE_CODE (var) == VAR_DECL
>                                  && !gimple_vdef (stmt));
>             gsi_replace (si, gimple_build_nop (), true);
>             register_new_def (get_or_create_ssa_default_def (cfun, var),
> var);
>             break;
>           }
>
>
> I'll be away next week, but I'll re-read all the replies carefully when I
> come back.
>
> --
> Marc Glisse


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