This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR70484, RTL DSE using wrong dependence check
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: Bernd Schmidt <bschmidt at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Mon, 4 Apr 2016 11:50:02 +0200
- Subject: Re: [PATCH] Fix PR70484, RTL DSE using wrong dependence check
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1604011106140 dot 13384 at t29 dot fhfr dot qr> <56FE931D dot 4030604 at redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1604041116510 dot 13384 at t29 dot fhfr dot qr>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Mon, Apr 04, 2016 at 11:24:41AM +0200, Richard Biener wrote:
> On Fri, 1 Apr 2016, Bernd Schmidt wrote:
>
> > On 04/01/2016 11:08 AM, Richard Biener wrote:
> > > {
> > > ! if (canon_true_dependence (s_info->mem,
> > > ! GET_MODE (s_info->mem),
> > > ! s_info->mem_addr,
> > > ! mem, mem_addr))
> > > {
> > > s_info->rhs = NULL;
> > > s_info->const_rhs = NULL;
> > > --- 1609,1617 ----
> > > the value of store_info. If it is, set the rhs to NULL to
> > > keep it from being used to remove a load. */
> > > {
> > > ! if (canon_output_dependence (s_info->mem, true,
> > > ! mem, GET_MODE (mem),
> > > ! mem_addr))
> > > {
> > > s_info->rhs = NULL;
> > > s_info->const_rhs = NULL;
> >
> > I think the patch is ok, but there is a comment in that function which
> > references canon_true_dependence; that should also be fixed up.
>
> Done, though I don't understand it at all ... if alias-set was supposed
> to be zero for all cases we call canon_true_dependence then the issue
> wouldn't have happened. Maybe there was times where passing mem_addr
> == NULL_RTX to canon_true_dependence caused it to bail out?
>
> Not sure how to adjust that comment now, maybe it would be valid
> to simply remove the if (spill_alias_set) case and always use
> the else case?
I believe all of the spill_alias_set stuff is dead for many years.
In 4.4, a call to dse_record_singleton_alias_set has been removed
(supposedly related to introduction of IRA). Then in 4.8 you've
removed the dse_record_singleton_alias_set function, later on Lawrence
removed other small bits of this.
E.g. the alias_set_out argument from canon_address, all of spill_alias_set
handling, alias_set field, clear_alias_set_lookup, clear_alias_mode_holder,
clear_alias_group, clear_alias_mode_table are all dead IMHO.
Jakub