[SVE] PR91532

[Prathamesh Kulkarni]

On Wed, 2 Oct 2019 at 01:08, Jeff Law <law@redhat.com> wrote:
>
> On 10/1/19 12:40 AM, Richard Biener wrote:
> > 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?
> I think we want to stop at the block boundary -- aren't the cases we
> care about here local to a block?
This version restricts walking in dse_classify_store to basic-block if
bb_only is true,
and removes dead stores in ifcvt_local_dce instead of separate walk.
Does it look OK ?

Thanks,
Prathamesh
>
> jeff
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr91532-2.diff
Type: text/x-patch
Size: 3903 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20191001/03b2633f/attachment.bin>