Re: [rfa] Fix PR44699: bootstrap problem

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

On Wed, Jun 30, 2010 at 3:29 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Jun 30, 2010 at 3:25 PM, Michael Matz <matz@suse.de> wrote:
>> Hi,
>>
>> On Tue, 29 Jun 2010, Richard Guenther wrote:
>>
>>> On Tue, Jun 29, 2010 at 5:45 PM, Michael Matz <matz@suse.de> wrote:
>>> > Hello,
>>> >
>>> > Allocating an array of length num_ssa_names, then doing foldings or other
>>> > transformations that can create new SSA names, and then iterating that
>>> > array again as if it had num_ssa_names elements is not a good idea.
>>> >
>>> > This fixes the testcase. ?I'm going to regstrap this, once the
>>> > function_arg_advance brokenness is fixed. ?Okay, assuming this will work?
>>>
>>> Ok.
>>
>> While it fixes the testcase (and regstraps on linux) it still breaks on
>> Darwin. ?We also need to be careful to not attach VDEFs to the generated
>> new 'lhs = tmp' assignment if LHS is a gimple reg. ?Those VDEFs would be
>> removed fairly quickly breaking the vdef/vuse chains again (that was
>> pre-existing breakage).
>>
>> So I'd like to add this hunk to the patch (whole patch regstrapping again
>> on x86_64-linux). ?Okay for trunk?
>>
>>
>> Ciao,
>> Michael.
>> --
>> ? ? ? ?* gimple-fold.c (gimplify_and_update_call_from_tree): If LHS is
>> ? ? ? ?a gimple reg, attach the original VDEF to the last store in the
>> ? ? ? ?sequence.
>>
>> 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);
>> + ? ? ? ? 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. ?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.

Thus, more like

Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c   (revision 161603)
+++ gcc/gimple-fold.c   (working copy)
@@ -1152,15 +1152,24 @@ gimplify_and_update_call_from_tree (gimp
        }
       if (laststore)
        {
-         reaching_vuse = make_ssa_name (gimple_vop (cfun), laststore);
+         if (!is_gimple_reg (lhs) && last != laststore)
+           reaching_vuse = make_ssa_name (gimple_vop (cfun), laststore);
+         else
+           {
+             reaching_vuse = gimple_vdef (stmt);
+             SSA_NAME_DEF_STMT (reaching_vuse) = laststore;
+           }
          gimple_set_vdef (laststore, reaching_vuse);
          update_stmt (laststore);
-         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 (TREE_CODE (lhs) == SSA_NAME)
+       SSA_NAME_DEF_STMT (lhs) = new_stmt;
+      if (!laststore)
+       {
+         gimple_set_vuse (new_stmt, reaching_vuse);
+         gimple_set_vdef (new_stmt, gimple_vdef (stmt));
+       }
     }

   gimple_set_location (new_stmt, gimple_location (stmt));

?



>
> RIchard.
>
>> + ? ? ?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));
>> + ? ? ? ? move_ssa_defining_stmt_for_defs (new_stmt, stmt);
>> + ? ? ? }
>> ? ? }
>>
>> ? gimple_set_location (new_stmt, gimple_location (stmt));
>