Re: [x86 PATCH take #3] PR target/91681: zero_extendditi2 pattern for more optimizations.

[Uros Bizjak <ubizjak@gmail.com>]

On Sat, Jul 23, 2022 at 10:51 AM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
>
> Hi Uros,
>
> This is the next iteration of the zero_extendditi2 patch last reviewed here:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596204.html
>
>
>
> [1] The sse.md changes were split out, reviewed, approved and committed.
>
> [2] The *concat splitters have been moved post-reload matching what we
>
> now do for many/most of the double word functionality.
>
> [3] As you recommend, these *concat splitters now use split_double_mode
>
> to “subreg” operand[0] into parts, via a new helper function that can also
>
> handle overlapping registers, and even use xchg for the rare case that a
>
> double word is constructed from its high and low parts, but the wrong
>
> way around.
>
>
>
> 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.  Ok for mainline?
>
>
>
> 2022-07-23  Roger Sayle  <roger@nextmovesoftware.com>
>
>             Uroš Bizjak  <ubizjak@gmail.com>
>
>
>
> gcc/ChangeLog
>
>         PR target/91681
>
>         * config/i386/i386-expand.cc (split_double_concat): A new helper
>
>         function for setting a double word value from two word values.
>
>         * config/i386/i386-protos.h (split_double_concat): Prototype here.
>
>         * config/i386/i386.md (zero_extendditi2): New define_insn_and_split.
>
>         (*add<dwi>3_doubleword_zext): New define_insn_and_split.
>
>         (*sub<dwi>3_doubleword_zext): New define_insn_and_split.
>
>         (*concat<mode><dwi>3_1): New define_insn_and_split replacing
>
>         previous define_split for implementing DST = (HI<<32)|LO as
>
>         pair of move instructions, setting lopart and hipart.
>
>         (*concat<mode><dwi>3_2): Likewise.
>
>         (*concat<mode><dwi>3_3): Likewise, where HI is zero_extended.
>
>         (*concat<mode><dwi>3_4): Likewise, where HI is zero_extended.
>
>
>
> gcc/testsuite/ChangeLog
>
>         PR target/91681
>
>         * g++.target/i386/pr91681.C: New test case (from the PR).
>
>         * gcc.target/i386/pr91681-1.c: New int128 test case.
>
>         * gcc.target/i386/pr91681-2.c: Likewise.
>
>         * gcc.target/i386/pr91681-3.c: Likewise, but for ia32.

OK with two small adjustments:

+(define_insn_and_split "zero_extendditi2"
+  [(set (match_operand:TI 0 "nonimmediate_operand" "=r,o")
+ (zero_extend:TI (match_operand:DI 1 "nonimmediate_operand" "rm,r")))]

Please put the new pattern above zero_extendsidi2.

+(define_insn_and_split "*add<dwi>3_doubleword_zext"
+  [(set (match_operand:<DWI> 0 "nonimmediate_operand" "=r,o")

And this one after *add<dwi>3_doubleword, to keep all _doubleword
patterns together.

Thanks,
Uros.