This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix PR 91708

On 9/13/19 1:23 PM, Richard Biener wrote:
> On Thu, 12 Sep 2019, Bernd Edlinger wrote:
>> On 9/12/19 10:08 AM, Richard Biener wrote:
>>> On Wed, 11 Sep 2019, Bernd Edlinger wrote:
>>>> On 9/11/19 8:30 PM, Richard Biener wrote:
>>> More like the following?  I wonder if we can assert that
>>> MEM_NOTRAP_P () are equal (see all the for_gcse checks in exp_equiv_p).
>>> But as said earlier I wonder in which cases a replacement is
>>> profitable at all - thus, if a
>>>   else if (MEM_P (trial))
>>>     /* Do not replace anything with a MEM.  */
>>>     ;
>> Yes, I like that better, since there is essentially nothing stopping
>> it from replacing a REG with a MEM, except the rtxcost function, maybe.
>> It turned out that the previous version got into an endless loop here,
>> since the loop termination happens when replacing MEM by itself, except
>> when something else terminates the search.
> Is that how it works without the patch as well?  I admit the loop
> is a bit hard to follow since it is not only iterating
> over elt->next_same_value ...

Yes, that seems to be the case, since the memory reference itself
is always in the set.  There was either an endless loop, when
the src variable was set this if can always be taken, which makes
no progress:

          else if (src
                   && preferable (src_cost, src_regcost,
                                  src_eqv_cost, src_eqv_regcost) <= 0
                   && preferable (src_cost, src_regcost,
                                  src_related_cost, src_related_regcost) <= 0
                   && preferable (src_cost, src_regcost,
                                  src_elt_cost, src_elt_regcost) <= 0)
            trial = src, src_cost = MAX_COST;

or there was a crash when the above was not taken then:

              trial = elt->exp;
              elt = elt->next_same_value;
              src_elt_cost = MAX_COST;

crashes, because elt == NULL at some point.

>> So how about this?
>> The only possible MEM->MEM transformations are now:
>> - replacing trial = dest (result mov dest, dest; which is a no-op insn)
>> - replacing trial = src (which is a no-op transformation, and exits the loop)
>> Therefore the overlapping mem move handling no longer necessary. 
>> Bootstrap and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
> Looks OK to me.  Can you see if this has any unexpected code-generation
> effects?  I would expect it to not make a difference at all, but you
> never know -- any differences in cc1files?

I tried to install the patch after _stage1_ was competed,
but there was no bootstrap miscompare (on x86_64-pc-linux-gnu)
so, no this does not make any difference here.


> Not really my primary area of expertise...
> Thanks,
> Richard.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]