Re: [PATCH] x86: Optimize VFIXUPIMM* patterns with multiple-alternative constraints
On Fri, Nov 9, 2018 at 6:05 PM Uros Bizjak wrote: > > On Fri, Nov 9, 2018 at 10:54 AM Wei Xiao wrote: > > > > Hi Uros > > > > Thanks for the remarks! > > I improve the patch as attached to address the issues you mentioned: > > 1. No changes to substs any more. > > 2. Adopt established approach (e.g "rcp14") > > to > > handle zero masks. > > > > I'd like to explain our motivation of combining vfixupimm patterns: there > > will > > be a lot of new x86 instructions with both masking and rounding like > > vfixupimm > > in the future but we still want to keep x86 MD as short as possible and > > don't > > want to write 2 patterns for each of these new instructions, which will also > > raise code review cost for maintainer. We want to make sure the new pattern > > paradigm is ok for x86 maintainer through this patch. > > Yes, the patch looks much nicer now. > > +2018-11-09 Wei Xiao > + *config/i386/sse.md: Combine VFIXUPIMM* patterns > + (_fixupimm_maskz): Update. > + (_fixupimm): Update. > + (_fixupimm_mask): Remove. > + (avx512f_sfixupimm_maskz): Update. > + (avx512f_sfixupimm): Update. > + (avx512f_sfixupimm_mask): Remove. > > (In future, please add ChangeLog entry to the text of the mail). > > OK for mainline. > > Thanks, > Uros. Thanks for the review. In future we will pay more attentions to follow the convention. BR, Terry
Re: [PATCH] x86: Optimize VFIXUPIMM* patterns with multiple-alternative constraints
On Fri, Nov 9, 2018 at 10:54 AM Wei Xiao wrote: > > Hi Uros > > Thanks for the remarks! > I improve the patch as attached to address the issues you mentioned: > 1. No changes to substs any more. > 2. Adopt established approach (e.g "rcp14") to > handle zero masks. > > I'd like to explain our motivation of combining vfixupimm patterns: there will > be a lot of new x86 instructions with both masking and rounding like vfixupimm > in the future but we still want to keep x86 MD as short as possible and don't > want to write 2 patterns for each of these new instructions, which will also > raise code review cost for maintainer. We want to make sure the new pattern > paradigm is ok for x86 maintainer through this patch. Yes, the patch looks much nicer now. +2018-11-09 Wei Xiao + *config/i386/sse.md: Combine VFIXUPIMM* patterns + (_fixupimm_maskz): Update. + (_fixupimm): Update. + (_fixupimm_mask): Remove. + (avx512f_sfixupimm_maskz): Update. + (avx512f_sfixupimm): Update. + (avx512f_sfixupimm_mask): Remove. (In future, please add ChangeLog entry to the text of the mail). OK for mainline. Thanks, Uros.
Re: [PATCH] x86: Optimize VFIXUPIMM* patterns with multiple-alternative constraints
Hi Uros Thanks for the remarks! I improve the patch as attached to address the issues you mentioned: 1. No changes to substs any more. 2. Adopt established approach (e.g "rcp14") to handle zero masks. I'd like to explain our motivation of combining vfixupimm patterns: there will be a lot of new x86 instructions with both masking and rounding like vfixupimm in the future but we still want to keep x86 MD as short as possible and don't want to write 2 patterns for each of these new instructions, which will also raise code review cost for maintainer. We want to make sure the new pattern paradigm is ok for x86 maintainer through this patch. Wei Uros Bizjak 于2018年11月7日周三 下午3:24写道： > > On Tue, Nov 6, 2018 at 11:16 AM Wei Xiao wrote: > > > > Hi maintainers, > > > > The attached patch intends to optimize VFIXUPIMM* patterns with > > multiple-alternative constraints and > > 4 patterns are combined into 2 patterns. Tested with bootstrap and > > regression tests on x86_64. No regressions. > > > > Is it OK for trunk? > > I'm not convinced that this particular optimization is a good idea. > Looking at the patch, you have to add a whole bunch of substs just to > merge two pattern sets. Also, the approach diverges from established > approach of handling zero masks. The later raises maintenance costs > for no compelling reason. > > I'd say to leave these patterns the way they are. > > Uros. combine-vfixupimm-v2.diff Description: Binary data
Re: [PATCH] x86: Optimize VFIXUPIMM* patterns with multiple-alternative constraints
On Tue, Nov 6, 2018 at 11:16 AM Wei Xiao wrote: > > Hi maintainers, > > The attached patch intends to optimize VFIXUPIMM* patterns with > multiple-alternative constraints and > 4 patterns are combined into 2 patterns. Tested with bootstrap and > regression tests on x86_64. No regressions. > > Is it OK for trunk? I'm not convinced that this particular optimization is a good idea. Looking at the patch, you have to add a whole bunch of substs just to merge two pattern sets. Also, the approach diverges from established approach of handling zero masks. The later raises maintenance costs for no compelling reason. I'd say to leave these patterns the way they are. Uros.
[PATCH] x86: Optimize VFIXUPIMM* patterns with multiple-alternative constraints
Hi maintainers, The attached patch intends to optimize VFIXUPIMM* patterns with multiple-alternative constraints and 4 patterns are combined into 2 patterns. Tested with bootstrap and regression tests on x86_64. No regressions. Is it OK for trunk? Thanks, Wei opt-vfixupimm-v1.diff Description: Binary data
