On Sun, May 26, 2013 at 5:53 AM, Easwaran Raman <era...@google.com> wrote:
> On Sat, May 25, 2013 at 4:46 AM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> Easwaran Raman <era...@google.com> wrote:
>>
>>>In that case, if my insert_stmt immediately follows dep_stmt and both
>>>have the same UID, not_dominated_by would return true and I will end
>>>up updating insert_stmt to dep_stmt which is wrong.
>>
>> But there should be a safe default answer for
>> Equal uids. Unless we are asking different questions in different places.
>> Thus, I do not like the stmt walking but rather have a safe fallback.
>
> I am lost here. I don't see how we could avoid doing the stmt walking
> to resolve the equal uid case. How to ensure that not_dominated_by (a,
> b) returns true and not_dominated_by (b, a) returns false if A and B
> have the same UID and A appears before B without doing the statement
> walk. And, I don't see why the statement walk is bad. It is not likely
> that there is a long sequence of statements with the same UID.
Sure, but if you are always asking a question like "is placing X before Y ok?"
then you can conservatively answer "no" and code should handle that ok.
If you are asking questions both way then of course no conservative answer is
possible. Both current uses of not_dominated_by are of the same kind,
if the placement is not ok then either the insert point needs adjustment or
the debug stmt reset.
Richard.
> Thanks,
> Easwaran
>
>>
>> Richard.
>>
>>>- Easwaran
>>>
>>>On Fri, May 24, 2013 at 1:07 AM, Richard Biener
>>><richard.guent...@gmail.com> wrote:
>>>> On Thu, May 23, 2013 at 7:26 PM, Easwaran Raman <era...@google.com>
>>>wrote:
>>>>> This addresses the case where UID alone is not sufficient to figure
>>>>> out which statement appears earlier in a BB. Bootstraps and no test
>>>>> regressions in x86_64 on linux. Ok for trunk?
>>>>
>>>> Why not simply conservatively use gimple_uid (a) <= gimple_uid (b)
>>>> in not_dominated_by?
>>>>
>>>> Richard.
>>>>
>>>>
>>>>
>>>>> Thanks,
>>>>> Easwaran
>>>>>
>>>>>
>>>>> 2013-05-23 Easwaran Raman <era...@google.com>
>>>>>
>>>>> PR tree-optimization/57337
>>>>> * tree-ssa-reassoc.c (appears_later_in_bb): New function.
>>>>> (find_insert_point): Correctly identify the insertion point
>>>>> when two statements with the same UID is compared.
>>>>>
>>>>> Index: gcc/tree-ssa-reassoc.c
>>>>> ===================================================================
>>>>> --- gcc/tree-ssa-reassoc.c (revision 199211)
>>>>> +++ gcc/tree-ssa-reassoc.c (working copy)
>>>>> @@ -2866,6 +2866,31 @@ not_dominated_by (gimple a, gimple b)
>>>>>
>>>>> }
>>>>>
>>>>> +/* Among STMT1 and STMT2, return the statement that appears later.
>>>Both
>>>>> + statements are in same BB and have the same UID. */
>>>>> +
>>>>> +static gimple
>>>>> +appears_later_in_bb (gimple stmt1, gimple stmt2)
>>>>> +{
>>>>> + unsigned uid = gimple_uid (stmt1);
>>>>> + gimple_stmt_iterator gsi = gsi_for_stmt (stmt1);
>>>>> + gsi_next (&gsi);
>>>>> + if (gsi_end_p (gsi))
>>>>> + return stmt1;
>>>>> + for (; !gsi_end_p (gsi); gsi_next (&gsi))
>>>>> + {
>>>>> + gimple stmt = gsi_stmt (gsi);
>>>>> +
>>>>> + /* If STMT has a different UID than STMT1 and we haven't seen
>>>>> + STMT2 during traversal, we know STMT1 appears later. */
>>>>> + if (gimple_uid (stmt) != uid)
>>>>> + return stmt1;
>>>>> + else if (stmt == stmt2)
>>>>> + return stmt2;
>>>>> + }
>>>>> + gcc_unreachable ();
>>>>> +}
>>>>> +
>>>>> /* Find the statement after which STMT must be moved so that the
>>>>> dependency from DEP_STMT to STMT is maintained. */
>>>>>
>>>>> @@ -2875,7 +2900,11 @@ find_insert_point (gimple stmt, gimple
>>>dep_stmt)
>>>>> gimple insert_stmt = stmt;
>>>>> if (dep_stmt == NULL)
>>>>> return stmt;
>>>>> - if (not_dominated_by (insert_stmt, dep_stmt))
>>>>> + if (gimple_uid (insert_stmt) == gimple_uid (dep_stmt)
>>>>> + && gimple_bb (insert_stmt) == gimple_bb (dep_stmt)
>>>>> + && insert_stmt != dep_stmt)
>>>>> + insert_stmt = appears_later_in_bb (insert_stmt, dep_stmt);
>>>>> + else if (not_dominated_by (insert_stmt, dep_stmt))
>>>>> insert_stmt = dep_stmt;
>>>>> return insert_stmt;
>>>>> }
>>
>>
