On Tue, Dec 13, 2022 at 10:20 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> The following patch fixes 2 issues with the *concat<half><mode>3_5 and
> *concat<mode><dwi>3_{6,7} patterns.
> One is that if the destination is memory rather than register, then
> we can't use movabsq and so can't support all the possible immediates.
> I see 3 possibilities to fix that. One would be to use
> x86_64_hilo_int_operand predicate instead of const_scalar_int_operand
> and thus not match it at all during combine in such cases, but that
> unnecessarily pessimizes also the case when it is loaded into register
> where we can use movabsq.
> Another one is what is implemented in the patch, use Wd constraint
> for the integer on 64-bit if destination is memory and X (didn't find
> more appropriate one which would accept any const_int/const_wide_int
> and the value checking is done in the conditions) otherwise.
Perhaps you should use "n" instead of "X".
> Yet another option would be to add match_scratch to the pattern and use
> it with =X constraints except for the =o case for 64-bit non-Wd where it
> would give a single DImode register (rather than 2).
>
> Another thing is that if one half of the constant is
> ix86_endbr_immediate_operand, then for -fcf-protection=branch we
> force those constants into memory and that might not work properly
> with -fpic. So we should refuse to match with such constants.
> OT, seems for movabsq we don't check that and happily allow the endbr
> pattern in the immediate.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk,
> or do you prefer another way to do it (see above)?
> 2022-12-13 Jakub Jelinek <ja...@redhat.com>
>
> PR target/108044
> * config/i386/i386.md (*concat<half><mode>3_5, *concat<mode><dwi>3_6,
> *concat<mode><dwi>3_7): Split alternative with =ro output constraint
> into =r,o,o and use Wd input constraint for the last alternative which
> is enabled for TARGET_64BIT. Reject ix86_endbr_immediate_operand
> in the input constant.
>
> * gcc.target/i386/pr108044-1.c: New test.
> * gcc.target/i386/pr108044-2.c: New test.
> * gcc.target/i386/pr108044-3.c: New test.
> * gcc.target/i386/pr108044-4.c: New test.
OK with or without the change to "n" constraint, although I would
prefer "n", since "X" can perhaps result in some yet unknown
surprises.
Thanks,
Uros.
> --- gcc/config/i386/i386.md.jj 2022-12-08 14:55:38.807303856 +0100
> +++ gcc/config/i386/i386.md 2022-12-12 10:37:09.332995296 +0100
> @@ -11470,11 +11470,11 @@ (define_insn_and_split "*concat<mode><dw
> })
>
> (define_insn_and_split "*concat<half><mode>3_5"
> - [(set (match_operand:DWI 0 "nonimmediate_operand" "=ro")
> + [(set (match_operand:DWI 0 "nonimmediate_operand" "=r,o,o")
> (any_or_plus:DWI
> - (ashift:DWI (match_operand:DWI 1 "register_operand" "r")
> + (ashift:DWI (match_operand:DWI 1 "register_operand" "r,r,r")
> (match_operand:DWI 2 "const_int_operand"))
> - (match_operand:DWI 3 "const_scalar_int_operand")))]
> + (match_operand:DWI 3 "const_scalar_int_operand" "X,X,Wd")))]
> "INTVAL (operands[2]) == <MODE_SIZE> * BITS_PER_UNIT / 2
> && (<MODE>mode == DImode
> ? CONST_INT_P (operands[3])
> @@ -11482,7 +11482,12 @@ (define_insn_and_split "*concat<half><mo
> : CONST_INT_P (operands[3])
> ? INTVAL (operands[3]) >= 0
> : CONST_WIDE_INT_NUNITS (operands[3]) == 2
> - && CONST_WIDE_INT_ELT (operands[3], 1) == 0)"
> + && CONST_WIDE_INT_ELT (operands[3], 1) == 0)
> + && !(CONST_INT_P (operands[3])
> + ? ix86_endbr_immediate_operand (operands[3], VOIDmode)
> + : ix86_endbr_immediate_operand (GEN_INT (CONST_WIDE_INT_ELT
> (operands[3],
> + 0)),
> + VOIDmode))"
> "#"
> "&& reload_completed"
> [(clobber (const_int 0))]
> @@ -11491,16 +11496,17 @@ (define_insn_and_split "*concat<half><mo
> split_double_concat (<MODE>mode, operands[0], op3,
> gen_lowpart (<HALF>mode, operands[1]));
> DONE;
> -})
> +}
> + [(set_attr "isa" "*,nox64,x64")])
>
> (define_insn_and_split "*concat<mode><dwi>3_6"
> - [(set (match_operand:<DWI> 0 "nonimmediate_operand" "=ro,r")
> + [(set (match_operand:<DWI> 0 "nonimmediate_operand" "=r,o,o,r")
> (any_or_plus:<DWI>
> (ashift:<DWI>
> (zero_extend:<DWI>
> - (match_operand:DWIH 1 "nonimmediate_operand" "r,m"))
> + (match_operand:DWIH 1 "nonimmediate_operand" "r,r,r,m"))
> (match_operand:<DWI> 2 "const_int_operand"))
> - (match_operand:<DWI> 3 "const_scalar_int_operand")))]
> + (match_operand:<DWI> 3 "const_scalar_int_operand" "X,X,Wd,X")))]
> "INTVAL (operands[2]) == <MODE_SIZE> * BITS_PER_UNIT
> && (<DWI>mode == DImode
> ? CONST_INT_P (operands[3])
> @@ -11508,7 +11514,12 @@ (define_insn_and_split "*concat<mode><dw
> : CONST_INT_P (operands[3])
> ? INTVAL (operands[3]) >= 0
> : CONST_WIDE_INT_NUNITS (operands[3]) == 2
> - && CONST_WIDE_INT_ELT (operands[3], 1) == 0)"
> + && CONST_WIDE_INT_ELT (operands[3], 1) == 0)
> + && !(CONST_INT_P (operands[3])
> + ? ix86_endbr_immediate_operand (operands[3], VOIDmode)
> + : ix86_endbr_immediate_operand (GEN_INT (CONST_WIDE_INT_ELT
> (operands[3],
> + 0)),
> + VOIDmode))"
> "#"
> "&& reload_completed"
> [(clobber (const_int 0))]
> @@ -11516,20 +11527,25 @@ (define_insn_and_split "*concat<mode><dw
> rtx op3 = simplify_subreg (<MODE>mode, operands[3], <DWI>mode, 0);
> split_double_concat (<DWI>mode, operands[0], op3, operands[1]);
> DONE;
> -})
> +}
> + [(set_attr "isa" "*,nox64,x64,*")])
>
> (define_insn_and_split "*concat<mode><dwi>3_7"
> - [(set (match_operand:<DWI> 0 "nonimmediate_operand" "=ro,r")
> + [(set (match_operand:<DWI> 0 "nonimmediate_operand" "=r,o,o,r")
> (any_or_plus:<DWI>
> (zero_extend:<DWI>
> - (match_operand:DWIH 1 "nonimmediate_operand" "r,m"))
> - (match_operand:<DWI> 2 "const_scalar_int_operand")))]
> + (match_operand:DWIH 1 "nonimmediate_operand" "r,r,r,m"))
> + (match_operand:<DWI> 2 "const_scalar_int_operand" "X,X,Wd,X")))]
> "<DWI>mode == DImode
> ? CONST_INT_P (operands[2])
> && (UINTVAL (operands[2]) & GET_MODE_MASK (SImode)) == 0
> + && !ix86_endbr_immediate_operand (operands[2], VOIDmode)
> : CONST_WIDE_INT_P (operands[2])
> && CONST_WIDE_INT_NUNITS (operands[2]) == 2
> - && CONST_WIDE_INT_ELT (operands[2], 0) == 0"
> + && CONST_WIDE_INT_ELT (operands[2], 0) == 0
> + && !ix86_endbr_immediate_operand (GEN_INT (CONST_WIDE_INT_ELT
> (operands[2],
> + 1)),
> + VOIDmode)"
> "#"
> "&& reload_completed"
> [(clobber (const_int 0))]
> @@ -11541,7 +11557,8 @@ (define_insn_and_split "*concat<mode><dw
> op2 = gen_int_mode (CONST_WIDE_INT_ELT (operands[2], 1), <MODE>mode);
> split_double_concat (<DWI>mode, operands[0], operands[1], op2);
> DONE;
> -})
> +}
> + [(set_attr "isa" "*,nox64,x64,*")])
>
> ;; Negation instructions
>
> --- gcc/testsuite/gcc.target/i386/pr108044-1.c.jj 2022-12-12
> 10:25:23.664131494 +0100
> +++ gcc/testsuite/gcc.target/i386/pr108044-1.c 2022-12-12 10:20:02.395740622
> +0100
> @@ -0,0 +1,33 @@
> +/* PR target/108044 */
> +/* { dg-do compile { target int128 } } */
> +/* { dg-options "-O2" } */
> +
> +static inline unsigned __int128
> +foo (unsigned long long x, unsigned long long y)
> +{
> + return ((unsigned __int128) x << 64) | y;
> +}
> +
> +void
> +bar (unsigned __int128 *p, unsigned long long x)
> +{
> + p[0] = foo (x, 0xdeadbeefcafebabeULL);
> +}
> +
> +void
> +baz (unsigned __int128 *p, unsigned long long x)
> +{
> + p[0] = foo (0xdeadbeefcafebabeULL, x);
> +}
> +
> +void
> +qux (unsigned __int128 *p, unsigned long long x)
> +{
> + p[0] = foo (x, 0xffffffffcafebabeULL);
> +}
> +
> +void
> +corge (unsigned __int128 *p, unsigned long long x)
> +{
> + p[0] = foo (0xffffffffcafebabeULL, x);
> +}
> --- gcc/testsuite/gcc.target/i386/pr108044-2.c.jj 2022-12-12
> 10:25:27.069082645 +0100
> +++ gcc/testsuite/gcc.target/i386/pr108044-2.c 2022-12-12 10:19:06.545541879
> +0100
> @@ -0,0 +1,21 @@
> +/* PR target/108044 */
> +/* { dg-do compile { target ia32 } } */
> +/* { dg-options "-O2" } */
> +
> +static inline unsigned long long
> +foo (unsigned int x, unsigned int y)
> +{
> + return ((unsigned long long) x << 32) | y;
> +}
> +
> +void
> +bar (unsigned long long *p, unsigned int x)
> +{
> + p[0] = foo (x, 0xcafebabeU);
> +}
> +
> +void
> +baz (unsigned long long *p, unsigned int x)
> +{
> + p[0] = foo (0xcafebabeU, x);
> +}
> --- gcc/testsuite/gcc.target/i386/pr108044-3.c.jj 2022-12-12
> 10:27:08.348629628 +0100
> +++ gcc/testsuite/gcc.target/i386/pr108044-3.c 2022-12-12 10:29:19.967741328
> +0100
> @@ -0,0 +1,33 @@
> +/* PR target/108044 */
> +/* { dg-do compile { target int128 } } */
> +/* { dg-options "-O2 -fcf-protection=branch" } */
> +
> +static inline unsigned __int128
> +foo (unsigned long long x, unsigned long long y)
> +{
> + return ((unsigned __int128) x << 64) | y;
> +}
> +
> +unsigned __int128
> +bar (unsigned long long x)
> +{
> + return foo (x, 0xfa1e0ff3ULL);
> +}
> +
> +unsigned __int128
> +baz (unsigned long long x)
> +{
> + return foo (0xfa1e0ff3ULL, x);
> +}
> +
> +unsigned __int128
> +qux (unsigned long long x)
> +{
> + return foo (x, 0xffbafa1e0ff3abdeULL);
> +}
> +
> +unsigned __int128
> +corge (unsigned long long x)
> +{
> + return foo (0xffbafa1e0ff3abdeULL, x);
> +}
> --- gcc/testsuite/gcc.target/i386/pr108044-4.c.jj 2022-12-12
> 10:37:54.419345499 +0100
> +++ gcc/testsuite/gcc.target/i386/pr108044-4.c 2022-12-12 10:38:43.668635715
> +0100
> @@ -0,0 +1,21 @@
> +/* PR target/108044 */
> +/* { dg-do compile { target ia32 } } */
> +/* { dg-options "-O2 -fcf-protection=branch" } */
> +
> +static inline unsigned long long
> +foo (unsigned int x, unsigned int y)
> +{
> + return ((unsigned long long) x << 32) | y;
> +}
> +
> +unsigned long long
> +bar (unsigned int x)
> +{
> + return foo (x, 0xfa1e0ff3U);
> +}
> +
> +unsigned long long
> +baz (unsigned int x)
> +{
> + return foo (0xfa1e0ff3U, x);
> +}
>
> Jakub
>
