[SVE] PR91532

[Richard Biener]

On Mon, 30 Sep 2019, Prathamesh Kulkarni wrote:

> On Wed, 25 Sep 2019 at 23:44, Richard Biener <rguenther@suse.de> wrote:
> >
> > On Wed, 25 Sep 2019, Prathamesh Kulkarni wrote:
> >
> > > On Fri, 20 Sep 2019 at 15:20, Jeff Law <law@redhat.com> wrote:
> > > >
> > > > On 9/19/19 10:19 AM, Prathamesh Kulkarni wrote:
> > > > > Hi,
> > > > > For PR91532, the dead store is trivially deleted if we place dse pass
> > > > > between ifcvt and vect. Would it be OK to add another instance of dse there ?
> > > > > Or should we add an ad-hoc "basic-block dse" sub-pass to ifcvt that
> > > > > will clean up the dead store ?
> > > > I'd hesitate to add another DSE pass.  If there's one nearby could we
> > > > move the existing pass?
> > > Well I think the nearest one is just after pass_warn_restrict. Not
> > > sure if it's a good
> > > idea to move it up from there ?
> >
> > You'll need it inbetween ifcvt and vect so it would be disabled
> > w/o vectorization, so no, that doesn't work.
> >
> > ifcvt already invokes SEME region value-numbering so if we had
> > MESE region DSE it could use that.  Not sure if you feel like
> > refactoring DSE to work on regions - it currently uses a DOM
> > walk which isn't suited for that.
> >
> > if-conversion has a little "local" dead predicate compute removal
> > thingy (not that I like that), eventually it can be enhanced to
> > do the DSE you want?  Eventually it should be moved after the local
> > CSE invocation though.
> Hi,
> Thanks for the suggestions.
> For now, would it be OK to do "dse" on loop header in
> tree_if_conversion, as in the attached patch ?
> The patch does local dse in a new function ifcvt_local_dse instead of
> ifcvt_local_dce, because it needed to be done after RPO VN which
> eliminates:
> Removing dead stmt _ifc__62 = *_55;
> and makes the following store dead:
> *_55 = _ifc__61;

I suggested trying to move ifcvt_local_dce after RPO VN, you could
try that as independent patch (pre-approved).

I don't mind the extra walk though.

What I see as possible issue is that dse_classify_store walks virtual
uses and I'm not sure if the loop exit is a natural boundary for
such walk (eventually the loop header virtual PHI is reached but
there may also be a loop-closed PHI for the virtual operand,
but not necessarily).  So the question is whether to add a
"stop at" argument to dse_classify_store specifying the virtual
use the walk should stop at?

Thanks,
Richard.