On Tue, Nov 8, 2022 at 4:51 PM 钟云德 <zhong_1985...@163.com> wrote:
> At 2022-11-08 22:58:34, "Richard Biener" <richard.guent...@gmail.com> > wrote: > > >On Sat, Nov 5, 2022 at 10:03 AM Zhongyunde via Gcc-patches > ><gcc-patches@gcc.gnu.org> wrote: > >> > >> > >> > -----Original Message----- > >> > From: Andrew Pinski [mailto:pins...@gcc.gnu.org] > >> > Sent: Saturday, November 5, 2022 2:34 PM > >> > To: Zhongyunde <zhongyu...@huawei.com> > >> > Cc: hongtao....@intel.com; gcc-patches@gcc.gnu.org; Zhangwen(Esan) > >> > <zwzhangwen.zh...@huawei.com>; Weiwei (weiwei, Compiler) > >> > <weiwe...@huawei.com>; zhong_1985...@163.com > >> > Subject: Re: [PATCH] [PHIOPT] Add A ? B + CST : B match and simplify > >> > optimizations > >> > > >> > On Fri, Nov 4, 2022 at 11:17 PM Zhongyunde <zhongyu...@huawei.com> > >> > wrote: > >> > > > >> > > hi, > >> > > This patch is try to fix the issue > >> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107190, > >> > > would you like to give me some suggestion, thanks. > >> > > >> > This seems like a "simplified" version of > >> > https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584411.html > >> > which just handles power of 2 constants where we know the cond will be > >> > removed. > >> > We could do even more "simplified" of 1 if needed really. > >> > What is the IR before PHI-OPT? Is it just + 1? > >> > >> Thanks for your attention. It is + 4294967296 before PHI-OPT (See detail > >> https://gcc.godbolt.org/z/6zEc6ja1z) > >> So we should keep matching the power of 2 constants ? > >> > >> > Also your pattern can be simplified to use integer_pow2p in the match > >> > part > >> > instead of INTEGER_CST. > >> > > >> Apply your comment, thanks > > > >How does the patch fix the mentioned bug? match.pd patterns should make > >things > >"simpler" but x + (a << C') isn't simpler than a ? x + C : x. It > >looks you are targeting > >PHI-OPT here, so maybe instead extend value_replacement to handle this case, > >it does look similar to the case with neutral/absorbing element there? > > > >Richard. > > Thanks. This patch try to fix the 1st issued mentioned in 107090 – [aarch64] > sequence logic should be combined with mul and umulh (gnu.org) > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107090> > Sure, I'll take a look at the function value_replacement. > I have also noticed that the function of two_value_replacement is very close > to patch I want to achieve, and it may be easy to extend. > It seems can be expressed equally in match.pd (called by > match_simplify_replacement), so how do we > choose where to implement may be better? > > I think that if we realize that we don't want a simplification to always apply (like by checking for canonicalize_math_p ()), then we should look for a pass which is a good fit and since you add the pattern to trigger a PHI-OPT optimization that looked like an obvious choice ... ``` > > /* Do the replacement of conditional if it can be done. */ > if (!early_p > > && !diamond_p > && > two_value_replacement (bb, bb1, e2, phi, arg0, arg1)) > cfgchanged = true; > > else if (!diamond_p > && > match_simplify_replacement (bb, bb1, e1, e2, phi, > arg0, > arg1, early_p)) > cfgchanged = true; > > ``` > >> > Thanks, >> > Andrew >> >> > >
