From patchwork Wed Jun 30 13:42:06 2010
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [rfa] Fix PR44699: bootstrap problem
Date: Wed, 30 Jun 2010 03:42:06 -0000
From: Richard Guenther <richard.guenther@gmail.com>
X-Patchwork-Id: 57412
Message-Id: <AANLkTimA7XdEpGyLyNHEK8STTTTlefOzt64tR7V2LvK0@mail.gmail.com>
To: Michael Matz <matz@suse.de>
Cc: gcc-patches@gcc.gnu.org

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


?



>
> 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));
>


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));