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!
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
csel5.patch
Description: csel5.patch
