Re: Re: [PATCH] [PHIOPT] Add A ? B + CST : B match and simplify optimizations
On Tue, Nov 8, 2022 at 4:51 PM 钟云德 wrote: > At 2022-11-08 22:58:34, "Richard Biener" > wrote: > > >On Sat, Nov 5, 2022 at 10:03 AM Zhongyunde via Gcc-patches > > wrote: > >> > >> > >> > -Original Message- > >> > From: Andrew Pinski [mailto:pins...@gcc.gnu.org] > >> > Sent: Saturday, November 5, 2022 2:34 PM > >> > To: Zhongyunde > >> > Cc: hongtao@intel.com; gcc-patches@gcc.gnu.org; Zhangwen(Esan) > >> > ; Weiwei (weiwei, Compiler) > >> > ; 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 > >> > 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 >> >> > >
Re:Re: [PATCH] [PHIOPT] Add A ? B + CST : B match and simplify optimizations
At 2022-11-08 22:58:34, "Richard Biener" wrote: >On Sat, Nov 5, 2022 at 10:03 AM Zhongyunde via Gcc-patches > wrote: >> >> >> > -Original Message- >> > From: Andrew Pinski [mailto:pins...@gcc.gnu.org] >> > Sent: Saturday, November 5, 2022 2:34 PM >> > To: Zhongyunde >> > Cc: hongtao@intel.com; gcc-patches@gcc.gnu.org; Zhangwen(Esan) >> > ; Weiwei (weiwei, Compiler) >> > ; 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 >> > 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 in107090 – [aarch64] sequence logic should be combined with mul and umulh (gnu.org) 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? ``` | /* 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; elseif (!diamond_p && match_simplify_replacement (bb, bb1, e1, e2, phi, arg0, arg1, early_p)) cfgchanged = true; | ``` >> > Thanks, >> > Andrew >> >>
Re: [PATCH] [PHIOPT] Add A ? B + CST : B match and simplify optimizations
On Sat, Nov 5, 2022 at 10:03 AM Zhongyunde via Gcc-patches wrote: > > > > -Original Message- > > From: Andrew Pinski [mailto:pins...@gcc.gnu.org] > > Sent: Saturday, November 5, 2022 2:34 PM > > To: Zhongyunde > > Cc: hongtao@intel.com; gcc-patches@gcc.gnu.org; Zhangwen(Esan) > > ; Weiwei (weiwei, Compiler) > > ; 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 > > 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, > > Andrew > >
RE: [PATCH] [PHIOPT] Add A ? B + CST : B match and simplify optimizations
> -Original Message- > From: Andrew Pinski [mailto:pins...@gcc.gnu.org] > Sent: Saturday, November 5, 2022 2:34 PM > To: Zhongyunde > Cc: hongtao@intel.com; gcc-patches@gcc.gnu.org; Zhangwen(Esan) > ; Weiwei (weiwei, Compiler) > ; 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 > 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 > Thanks, > Andrew 0001-PHIOPT-Add-A-B-op-CST-B-match-and-simplify-optimizat.patch Description: 0001-PHIOPT-Add-A-B-op-CST-B-match-and-simplify-optimizat.patch
Re: [PATCH] [PHIOPT] Add A ? B + CST : B match and simplify optimizations
On Fri, Nov 4, 2022 at 11:17 PM Zhongyunde 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? Also your pattern can be simplified to use integer_pow2p in the match part instead of INTEGER_CST. Thanks, Andrew > > ~/source/gccUpstreamDir/gcc/testsuite(cfg) » git format-patch -1 > --start-number=00 HEAD -o ~/patch > /home/zhongyunde/patch/-PHIOPT-Add-A-B-CST-B-match-and-simplify-optimization.patch
[PATCH] [PHIOPT] Add A ? B + CST : B match and simplify optimizations
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. ~/source/gccUpstreamDir/gcc/testsuite(cfg) » git format-patch -1 --start-number=00 HEAD -o ~/patch /home/zhongyunde/patch/-PHIOPT-Add-A-B-CST-B-match-and-simplify-optimization.patch -PHIOPT-Add-A-B-CST-B-match-and-simplify-optimization.patch Description: -PHIOPT-Add-A-B-CST-B-match-and-simplify-optimization.patch
