Re: [rfa] Fix PR44699: bootstrap problem

[Richard Guenther <richard dot guenther at gmail dot com>]

On Wed, Jun 30, 2010 at 3:53 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Wed, 30 Jun 2010, Richard Guenther wrote:
>
>> > - ? ? ?if (laststore)
>> > + ? ? ?if (laststore && is_gimple_reg (lhs))
>> > + ? ? ? {
>> > + ? ? ? ? gimple_set_vdef (laststore, gimple_vdef (stmt));
>> > + ? ? ? ? update_stmt (laststore);
>> > + ? ? ? ? move_ssa_defining_stmt_for_defs (laststore, stmt);
>> > + ? ? ? ? laststore = NULL;
>> > + ? ? ? }
>>
>> I don't think using move_ssa_defining_stmt_for_defs is appropriate
>> in this function.
>
> While it is pre-existing confusion in this function you're right. ?See
> below for the fix (just replacing move_ssa_defining_stmt_for_defs by
> SSA_NAME_DEF_STMT() assigns is enough).
>
>> It will move both virtual and real operand defs.
>> Instead the real defs (well, that for the lhs only) should be moved
>> to the last stmt in the sequence and the virtual def to the last store.
>>
>> The fn also now looks ugly.
>
> For sure. ?Can I make it nice later and fix the bootstrap problem on
> Darwin first? ?The freeze starts soon.

Yes, that works for me.

Richard.

> (FWIW the improvement you sent is not equivalent, it will attach a VDEF to
> the last insn when non is needed, and there's some confusion with the
> last==laststore test).
>
>
> Ciao,
> Michael.
> --
> Index: gimple-fold.c
> ===================================================================
> --- gimple-fold.c ? ? ? (revision 161602)
> +++ gimple-fold.c ? ? ? (working copy)
> @@ -1150,7 +1150,14 @@ gimplify_and_update_call_from_tree (gimp
> ? ? ? ? ?gsi_insert_before (si_p, last, GSI_NEW_STMT);
> ? ? ? ? ?gsi_next (si_p);
> ? ? ? ?}
> - ? ? ?if (laststore)
> + ? ? ?if (laststore && is_gimple_reg (lhs))
> + ? ? ? {
> + ? ? ? ? gimple_set_vdef (laststore, gimple_vdef (stmt));
> + ? ? ? ? update_stmt (laststore);
> + ? ? ? ? SSA_NAME_DEF_STMT (gimple_vdef (stmt)) = laststore;
> + ? ? ? ? laststore = NULL;
> + ? ? ? }
> + ? ? ?else if (laststore)
> ? ? ? ?{
> ? ? ? ? ?reaching_vuse = make_ssa_name (gimple_vop (cfun), laststore);
> ? ? ? ? ?gimple_set_vdef (laststore, reaching_vuse);
> @@ -1158,9 +1165,13 @@ gimplify_and_update_call_from_tree (gimp
> ? ? ? ? ?laststore = NULL;
> ? ? ? ?}
> ? ? ? new_stmt = gimple_build_assign (lhs, tmp);
> - ? ? ?gimple_set_vuse (new_stmt, reaching_vuse);
> - ? ? ?gimple_set_vdef (new_stmt, gimple_vdef (stmt));
> - ? ? ?move_ssa_defining_stmt_for_defs (new_stmt, stmt);
> + ? ? ?if (!is_gimple_reg (tmp))
> + ? ? ? gimple_set_vuse (new_stmt, reaching_vuse);
> + ? ? ?if (!is_gimple_reg (lhs))
> + ? ? ? {
> + ? ? ? ? gimple_set_vdef (new_stmt, gimple_vdef (stmt));
> + ? ? ? ? SSA_NAME_DEF_STMT (gimple_vdef (stmt)) = new_stmt;
> + ? ? ? }
> ? ? }
>
> ? gimple_set_location (new_stmt, gimple_location (stmt));