On Fri, Oct 6, 2023 at 3:59 PM Roger Sayle <ro...@nextmovesoftware.com> wrote:
>
>
> Grr! I've done it again. ENOPATCH.
>
> > -----Original Message-----
> > From: Roger Sayle <ro...@nextmovesoftware.com>
> > Sent: 06 October 2023 14:58
> > To: 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> > Cc: 'Uros Bizjak' <ubiz...@gmail.com>
> > Subject: [X86 PATCH] Implement doubleword right shifts by 1 bit using
> s[ha]r+rcr.
> >
> >
> > This patch tweaks the i386 back-end's ix86_split_ashr and ix86_split_lshr
> > functions to implement doubleword right shifts by 1 bit, using a shift of
> the
> > highpart that sets the carry flag followed by a rotate-carry-right
> > (RCR) instruction on the lowpart.
> >
> > Conceptually this is similar to the recent left shift patch, but with two
> > complicating factors. The first is that although the RCR sequence is
> shorter, and is
> > a ~3x performance improvement on AMD, my micro-benchmarking shows it
> > ~10% slower on Intel. Hence this patch also introduces a new
> > X86_TUNE_USE_RCR tuning parameter. The second is that I believe this is
> the
> > first time a "rotate-right-through-carry" and a right shift that sets the
> carry flag
> > from the least significant bit has been modelled in GCC RTL (on a MODE_CC
> > target). For this I've used the i386 back-end's UNSPEC_CC_NE which seems
> > appropriate. Finally rcrsi2 and rcrdi2 are separate define_insns so that
> we can
> > use their generator functions.
> >
> > For the pair of functions:
> > unsigned __int128 foo(unsigned __int128 x) { return x >> 1; }
> > __int128 bar(__int128 x) { return x >> 1; }
> >
> > with -O2 -march=znver4 we previously generated:
> >
> > foo: movq %rdi, %rax
> > movq %rsi, %rdx
> > shrdq $1, %rsi, %rax
> > shrq %rdx
> > ret
> > bar: movq %rdi, %rax
> > movq %rsi, %rdx
> > shrdq $1, %rsi, %rax
> > sarq %rdx
> > ret
> >
> > with this patch we now generate:
> >
> > foo: movq %rsi, %rdx
> > movq %rdi, %rax
> > shrq %rdx
> > rcrq %rax
> > ret
> > bar: movq %rsi, %rdx
> > movq %rdi, %rax
> > sarq %rdx
> > rcrq %rax
> > ret
> >
> > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and
> > make -k check, both with and without --target_board=unix{-m32} with no new
> > failures. And to provide additional testing, I've also bootstrapped and
> regression
> > tested a version of this patch where the RCR is always generated
> (independent of
> > the -march target) again with no regressions. Ok for mainline?
> >
> >
> > 2023-10-06 Roger Sayle <ro...@nextmovesoftware.com>
> >
> > gcc/ChangeLog
> > * config/i386/i386-expand.c (ix86_split_ashr): Split shifts by
> > one into ashr[sd]i3_carry followed by rcr[sd]i2, if TARGET_USE_RCR
> > or -Oz.
> > (ix86_split_lshr): Likewise, split shifts by one bit into
> > lshr[sd]i3_carry followed by rcr[sd]i2, if TARGET_USE_RCR or -Oz.
> > * config/i386/i386.h (TARGET_USE_RCR): New backend macro.
> > * config/i386/i386.md (rcrsi2): New define_insn for rcrl.
> > (rcrdi2): New define_insn for rcrq.
> > (<anyshiftrt><mode>3_carry): New define_insn for right shifts that
> > set the carry flag from the least significant bit, modelled using
> > UNSPEC_CC_NE.
> > * config/i386/x86-tune.def (X86_TUNE_USE_RCR): New tuning
> parameter
> > controlling use of rcr 1 vs. shrd, which is significantly faster
> on
> > AMD processors.
> >
> > gcc/testsuite/ChangeLog
> > * gcc.target/i386/rcr-1.c: New 64-bit test case.
> > * gcc.target/i386/rcr-2.c: New 32-bit test case.
OK.
Just don't set the new tune for generic. I hope Intel people notice
the performance difference...
Thanks,
Uros.
