On Fri, 13 Sep 2019, Bernd Edlinger wrote:
> 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:
>
> else
> {
> 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.
The patch is OK for trunk then.
Thanks,
Richard.
