On Mon, Jul 8, 2019 at 12:39 AM JiangNing OS
<jiangn...@os.amperecomputing.com> wrote:
>
> Hi Jeff and Richard B.,
>
> Following your tips, I've found a much simpler solution in tree-ssa-phiopt.c.
> Attached is the new patch. Review again, please!
/* Prove that we can move the store down. We could also check
TREE_THIS_NOTRAP here, but in that case we also could move stores,
whose value is not available readily, which we want to avoid. */
I think the comment above the change needs to be changed or extended slightly.
Thanks,
Andrew Pinski
>
> Thanks a lot!
> -Jiangning
>
> > -----Original Message-----
> > From: Jeff Law <l...@redhat.com>
> > Sent: Saturday, June 22, 2019 12:10 AM
> > To: Richard Biener <richard.guent...@gmail.com>
> > Cc: JiangNing OS <jiangn...@os.amperecomputing.com>; gcc-
> > patc...@gcc.gnu.org
> > Subject: Re: [PATCH] improve ifcvt optimization (PR rtl-optimization/89430)
> >
> > On 6/21/19 6:23 AM, Richard Biener wrote:
> > >
> > > That looks like a pretty easy form to analyze. I'd suggest looking
> > > through tree-ssa-phiopt.c closely. There's several transformations in
> > > there that share similarities with yours.
> > >
> > >
> > > More specifically there is the conditional store elimination (cselim)
> > > pass inside this file.
> > That's the one I was thinking of. It looks reasonably close to the cases
> > JiangNing is trying to capture.
> >
> >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >> 2) My current solution fits into current back-end if-conversion
> > > pass very well. I don't want to invent
> > >> a new framework to solve this relatively small issue. Besides,
> > > this back-end patch doesn't only
> > >> enhance store speculation detection, but also fix a bug in the
> > > original code. Understood, but I still wonder if we're better off
> > > addressing this in gimple.
> > >
> > >
> > > Traditionally if-conversions profitability heavily depends on the
> > > target and esp. if memory is involved costing on GIMPLE is hard.
> > > That's also one reason why we turned off loop if-conversion on GIMPLE
> > > for non-vectorized code.
> > Yea. That's always the concern for transformations that aren't trivial to
> > show
> > are better.
> >
> > Conditional store elimination as implemented right now in phiopt requires a
> > single store in the then/else clauses. So we're really just sinking the
> > stores
> > through a PHI. We're really not changing the number of loads or stores on
> > any path.
> >
> > In the cases I think JiangNing is trying to handle we are adding a store on
> > a
> > path where it didn't previously exist because there is no else clause. So
> > we
> > likely need to check the edge probabilities to avoid doing something dumb.
> > I
> > don't have a good sense where the cutoff should be. tree-ssa-sink might
> > have some nuggets of information to help guide.
> >
> > >
> > > phiopt is really about followup optimization opportunities in passes
> > > that do not handle conditionals well.
> > >
> > > cselim is on the border...
> > ACK. In fact, looking at it it I wonder if it'd be better in
> > tree-ssa-sink.c :-)
> >
> >
> > jeff
