Re: [PR71252][PR71269] Fix trunk errors due to stmt_to_insert

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

On Thu, May 26, 2016 at 11:32 AM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Jakub,
>
>
> On 26 May 2016 at 18:18, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Thu, May 26, 2016 at 02:17:56PM +1000, Kugan Vivekanandarajah wrote:
>>> --- a/gcc/tree-ssa-reassoc.c
>>> +++ b/gcc/tree-ssa-reassoc.c
>>> @@ -3767,8 +3767,10 @@ swap_ops_for_binary_stmt (vec<operand_entry *> ops,
>>>        operand_entry temp = *oe3;
>>>        oe3->op = oe1->op;
>>>        oe3->rank = oe1->rank;
>>> +      oe3->stmt_to_insert = oe1->stmt_to_insert;
>>>        oe1->op = temp.op;
>>>        oe1->rank= temp.rank;
>>> +      oe1->stmt_to_insert = temp.stmt_to_insert;
>>
>> If you want to swap those 3 fields (what about the others?), can't you write
>>       std::swap (oe1->op, oe3->op);
>>       std::swap (oe1->rank, oe3->rank);
>>       std::swap (oe1->stmt_to_insert, oe3->stmt_to_insert);
>> instead and drop operand_entry temp = *oe3; ?
>>
>>>      }
>>>    else if ((oe1->rank == oe3->rank
>>>           && oe2->rank != oe3->rank)
>>> @@ -3779,8 +3781,10 @@ swap_ops_for_binary_stmt (vec<operand_entry *> ops,
>>>        operand_entry temp = *oe2;
>>>        oe2->op = oe1->op;
>>>        oe2->rank = oe1->rank;
>>> +      oe2->stmt_to_insert = oe1->stmt_to_insert;
>>>        oe1->op = temp.op;
>>>        oe1->rank = temp.rank;
>>> +      oe1->stmt_to_insert = temp.stmt_to_insert;
>>>      }
>>
>> Similarly.
>
> Done. Revised patch attached.

Your patch only adds a single testcase, please make sure to include
_all_ relevant testcases.

The swap should simply swap the whole operand, thus

 std::swap (*oe1, *oe3);

it was probably not updated when all the other fields were added.

I don't like the find_insert_point changes or the change before
build_and_add_sum.
Why not move the if (stmt1) insert; if (stmt2) insert; before the if
() unconditionally?

Do we make progress with just the rest of the changes?  If so please split the
patch and include relevant testcases.

Thanks,
Richard.

> Thanks,
> Kugan