On Mon, 30 Sep 2019, Prathamesh Kulkarni wrote: > On Wed, 25 Sep 2019 at 23:44, Richard Biener <rguent...@suse.de> wrote: > > > > On Wed, 25 Sep 2019, Prathamesh Kulkarni wrote: > > > > > On Fri, 20 Sep 2019 at 15:20, Jeff Law <l...@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.
