On Fri, Jan 17, 2025 at 4:38 AM Hongyu Wang <hongyu.w...@intel.com> wrote:
>
> Hi,
>
> For shld/shrd_ndd_2 insn, the spiltter outputs wrong pattern that
> mixed parallel for clobber and set. Separate out the set to dest
> from parallel to fix it.
>
> Bootstrapped & regtested on x86-64-pc-linux-gnu.
>
> Ok for trunk?
>
> gcc/ChangeLog:
>
> PR target/118510
> * config/i386/i386.md (*x86_64_shld_ndd_2): Separate set to
> operand[0] from parallel in output template.
> (*x86_shld_ndd_2): Likewise.
> (*x86_64_shrd_ndd_2): Likewise.
> (*x86_shrd_ndd_2): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> PR target/118510
> * gcc.target/i386/pr118510.c: New test.
> ---
> gcc/config/i386/i386.md | 16 ++++++++--------
> gcc/testsuite/gcc.target/i386/pr118510.c | 14 ++++++++++++++
> 2 files changed, 22 insertions(+), 8 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr118510.c
>
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index 362b0ddcf40..ebfd76593ab 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -15615,8 +15615,8 @@ (define_insn_and_split "*x86_64_shld_ndd_2"
> (minus:QI (const_int 64)
> (and:QI (match_dup 3)
> (const_int 63)))) 0)))
> - (clobber (reg:CC FLAGS_REG))
> - (set (match_dup 0) (match_dup 4))])]
> + (clobber (reg:CC FLAGS_REG))])
> + (set (match_dup 0) (match_dup 4))]
Is there a reason to have operand 0 with "nonimmediate_operand"
predicate? If you have to generate a register temporary and then
unconditionally copy it to the output, it is better to use
"register_operand" predicate and leave middle end to do the copy for
you. Please see the patch at comment #3 in the PR.
Uros.
> {
> operands[4] = gen_reg_rtx (DImode);
> emit_move_insn (operands[4], operands[0]);
> @@ -15851,8 +15851,8 @@ (define_insn_and_split "*x86_shld_ndd_2"
> (minus:QI (const_int 32)
> (and:QI (match_dup 3)
> (const_int 31)))) 0)))
> - (clobber (reg:CC FLAGS_REG))
> - (set (match_dup 0) (match_dup 4))])]
> + (clobber (reg:CC FLAGS_REG))])
> + (set (match_dup 0) (match_dup 4))]
> {
> operands[4] = gen_reg_rtx (SImode);
> emit_move_insn (operands[4], operands[0]);
> @@ -17010,8 +17010,8 @@ (define_insn_and_split "*x86_64_shrd_ndd_2"
> (minus:QI (const_int 64)
> (and:QI (match_dup 3)
> (const_int 63)))) 0)))
> - (clobber (reg:CC FLAGS_REG))
> - (set (match_dup 0) (match_dup 4))])]
> + (clobber (reg:CC FLAGS_REG))])
> + (set (match_dup 0) (match_dup 4))]
> {
> operands[4] = gen_reg_rtx (DImode);
> emit_move_insn (operands[4], operands[0]);
> @@ -17245,8 +17245,8 @@ (define_insn_and_split "*x86_shrd_ndd_2"
> (minus:QI (const_int 32)
> (and:QI (match_dup 3)
> (const_int 31)))) 0)))
> - (clobber (reg:CC FLAGS_REG))
> - (set (match_dup 0) (match_dup 4))])]
> + (clobber (reg:CC FLAGS_REG))])
> + (set (match_dup 0) (match_dup 4))]
> {
> operands[4] = gen_reg_rtx (SImode);
> emit_move_insn (operands[4], operands[0]);
> diff --git a/gcc/testsuite/gcc.target/i386/pr118510.c
> b/gcc/testsuite/gcc.target/i386/pr118510.c
> new file mode 100644
> index 00000000000..6cfe8182b6f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr118510.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile { target { ! ia32 } } } */
> +/* { dg-options "-O2 -mapxf" } */
> +
> +typedef struct cpp_num cpp_num;
> +struct cpp_num {
> + int high;
> + unsigned low;
> + int overflow;
> +};
> +int num_rshift_n;
> +cpp_num num_lshift(cpp_num num) {
> + num.low = num.low >> num_rshift_n | num.high << (32 - num_rshift_n);
> + return num;
> +}
> --
> 2.31.1
>
