On Sat, Jul 12, 2014 at 8:15 AM, Marc Glisse <[email protected]> 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
