diff mbox series

Fix mismatch between constraint and predicate for ashl<mode>3_doubleword.

Message ID 20240730030453.853911-1-hongtao.liu@intel.com
State New
Headers show
Series Fix mismatch between constraint and predicate for ashl<mode>3_doubleword. | expand

Commit Message

liuhongt July 30, 2024, 3:04 a.m. UTC 
(insn 98 94 387 2 (parallel [
            (set (reg:TI 337 [ _32 ])
                (ashift:TI (reg:TI 329)
                    (reg:QI 521)))
            (clobber (reg:CC 17 flags))
        ]) "test.c":11:13 953 {ashlti3_doubleword}

is reloaded into

(insn 98 452 387 2 (parallel [
            (set (reg:TI 0 ax [orig:337 _32 ] [337])
                (ashift:TI (const_int 1671291085 [0x639de0cd])
                    (reg:QI 2 cx [521])))
            (clobber (reg:CC 17 flags))

since constraint n in the pattern accepts that.
(Not sure why reload doesn't check predicate)

(define_insn "ashl<mode>3_doubleword"
  [(set (match_operand:DWI 0 "register_operand" "=&r,&r")
        (ashift:DWI (match_operand:DWI 1 "reg_or_pm1_operand" "0n,r")
                    (match_operand:QI 2 "nonmemory_operand" "<S>c,<S>c")))

The patch fixes the mismatch between constraint and predicate.

Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
Ok for trunk?

gcc/ChangeLog:

	PR target/116096
	* config/i386/constraints.md (Wc): New constraint for integer
	1 or -1.
	* config/i386/i386.md (ashl<mode>3_doubleword): Refine
	constraint with Wc.

gcc/testsuite/ChangeLog:

	* gcc.target/i386/pr116096.c: New test.
---
 gcc/config/i386/constraints.md           |  6 ++++++
 gcc/config/i386/i386.md                  |  2 +-
 gcc/testsuite/gcc.target/i386/pr116096.c | 26 ++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr116096.c

Comments

Hongtao Liu Aug. 1, 2024, 6:14 a.m. UTC | #1 
On Tue, Jul 30, 2024 at 11:04 AM liuhongt <hongtao.liu@intel.com> wrote:
>
> (insn 98 94 387 2 (parallel [
>             (set (reg:TI 337 [ _32 ])
>                 (ashift:TI (reg:TI 329)
>                     (reg:QI 521)))
>             (clobber (reg:CC 17 flags))
>         ]) "test.c":11:13 953 {ashlti3_doubleword}
>
> is reloaded into
>
> (insn 98 452 387 2 (parallel [
>             (set (reg:TI 0 ax [orig:337 _32 ] [337])
>                 (ashift:TI (const_int 1671291085 [0x639de0cd])
>                     (reg:QI 2 cx [521])))
>             (clobber (reg:CC 17 flags))
>
> since constraint n in the pattern accepts that.
> (Not sure why reload doesn't check predicate)
>
> (define_insn "ashl<mode>3_doubleword"
>   [(set (match_operand:DWI 0 "register_operand" "=&r,&r")
>         (ashift:DWI (match_operand:DWI 1 "reg_or_pm1_operand" "0n,r")
>                     (match_operand:QI 2 "nonmemory_operand" "<S>c,<S>c")))
>
> The patch fixes the mismatch between constraint and predicate.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk?
>
> gcc/ChangeLog:
>
>         PR target/116096
>         * config/i386/constraints.md (Wc): New constraint for integer
>         1 or -1.
>         * config/i386/i386.md (ashl<mode>3_doubleword): Refine
>         constraint with Wc.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/i386/pr116096.c: New test.
> ---
>  gcc/config/i386/constraints.md           |  6 ++++++
>  gcc/config/i386/i386.md                  |  2 +-
>  gcc/testsuite/gcc.target/i386/pr116096.c | 26 ++++++++++++++++++++++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr116096.c
>
> diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
> index 7508d7a58bd..154cbccd09e 100644
> --- a/gcc/config/i386/constraints.md
> +++ b/gcc/config/i386/constraints.md
> @@ -254,6 +254,12 @@ (define_constraint "Wb"
>    (and (match_code "const_int")
>         (match_test "IN_RANGE (ival, 0, 7)")))
>
> +(define_constraint "Wc"
> +  "Integer constant -1 or 1."
> +  (and (match_code "const_int")
> +       (ior (match_test "op == constm1_rtx")
> +           (match_test "op == const1_rtx"))))
> +
>  (define_constraint "Ww"
>    "Integer constant in the range 0 @dots{} 15, for 16-bit shifts."
>    (and (match_code "const_int")
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index 6207036a2a0..79d5de5b46a 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -14774,7 +14774,7 @@ (define_insn_and_split "*ashl<dwi>3_doubleword_mask_1"
>
>  (define_insn "ashl<mode>3_doubleword"
>    [(set (match_operand:DWI 0 "register_operand" "=&r,&r")
> -       (ashift:DWI (match_operand:DWI 1 "reg_or_pm1_operand" "0n,r")
> +       (ashift:DWI (match_operand:DWI 1 "reg_or_pm1_operand" "0Wc,r")
>                     (match_operand:QI 2 "nonmemory_operand" "<S>c,<S>c")))
>     (clobber (reg:CC FLAGS_REG))]
>    ""
> diff --git a/gcc/testsuite/gcc.target/i386/pr116096.c b/gcc/testsuite/gcc.target/i386/pr116096.c
> new file mode 100644
> index 00000000000..5ef39805f58
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr116096.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile { target int128 } } */
> +/* { dg-options "-O2 -flive-range-shrinkage -fno-peephole2 -mstackrealign -Wno-psabi" } */
> +
> +typedef char U __attribute__((vector_size (32)));
> +typedef unsigned V __attribute__((vector_size (32)));
> +typedef __int128 W __attribute__((vector_size (32)));
> +U g;
> +
> +W baz ();
> +
> +static inline U
> +bar (V x, W y)
> +{
> +  y = y | y << (W) x;
> +  return (U)y;
> +}
> +
> +void
> +foo (W w)
> +{
> +  g = g <<
> +    bar ((V){baz ()[1], 3, 3, 5, 7},
> +        (W){w[0], ~(int) 2623676210}) >>
> +    bar ((V){baz ()[1]},
> +        (W){-w[0], ~(int) 2623676210});
> +}
> --
> 2.31.1
>
Uros Bizjak Aug. 1, 2024, 7:55 a.m. UTC | #2 
On Tue, Jul 30, 2024 at 5:05 AM liuhongt <hongtao.liu@intel.com> wrote:
>
> (insn 98 94 387 2 (parallel [
>             (set (reg:TI 337 [ _32 ])
>                 (ashift:TI (reg:TI 329)
>                     (reg:QI 521)))
>             (clobber (reg:CC 17 flags))
>         ]) "test.c":11:13 953 {ashlti3_doubleword}
>
> is reloaded into
>
> (insn 98 452 387 2 (parallel [
>             (set (reg:TI 0 ax [orig:337 _32 ] [337])
>                 (ashift:TI (const_int 1671291085 [0x639de0cd])
>                     (reg:QI 2 cx [521])))
>             (clobber (reg:CC 17 flags))
>
> since constraint n in the pattern accepts that.
> (Not sure why reload doesn't check predicate)

This is how reload works. It doesn't look at predicates, only at
constraints. To avoid checking errors in later passes, the predicate
should allow a superset of operands compared to the operands of the
constraint. Basically, reload is "refining" the operands to fit
constraints.

OTOH, predicates are used by pre-reload passes, e.g. combine to
combine various instructions. This is the reason sometimes insn has
nonimmediate_operand predicate and "r" constraint - to allow insn
combination while expecting that reload will fix the operand to fit
the constraint. Post-reload passes check both, predicats and
constraints, this is where you get failures due to mismatched reload
operand.

> (define_insn "ashl<mode>3_doubleword"
>   [(set (match_operand:DWI 0 "register_operand" "=&r,&r")
>         (ashift:DWI (match_operand:DWI 1 "reg_or_pm1_operand" "0n,r")
>                     (match_operand:QI 2 "nonmemory_operand" "<S>c,<S>c")))
>
> The patch fixes the mismatch between constraint and predicate.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk?
>
> gcc/ChangeLog:
>
>         PR target/116096
>         * config/i386/constraints.md (Wc): New constraint for integer
>         1 or -1.
>         * config/i386/i386.md (ashl<mode>3_doubleword): Refine
>         constraint with Wc.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/i386/pr116096.c: New test.

OK.

Thanks,
Uros.

> ---
>  gcc/config/i386/constraints.md           |  6 ++++++
>  gcc/config/i386/i386.md                  |  2 +-
>  gcc/testsuite/gcc.target/i386/pr116096.c | 26 ++++++++++++++++++++++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr116096.c
>
> diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
> index 7508d7a58bd..154cbccd09e 100644
> --- a/gcc/config/i386/constraints.md
> +++ b/gcc/config/i386/constraints.md
> @@ -254,6 +254,12 @@ (define_constraint "Wb"
>    (and (match_code "const_int")
>         (match_test "IN_RANGE (ival, 0, 7)")))
>
> +(define_constraint "Wc"
> +  "Integer constant -1 or 1."
> +  (and (match_code "const_int")
> +       (ior (match_test "op == constm1_rtx")
> +           (match_test "op == const1_rtx"))))
> +
>  (define_constraint "Ww"
>    "Integer constant in the range 0 @dots{} 15, for 16-bit shifts."
>    (and (match_code "const_int")
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index 6207036a2a0..79d5de5b46a 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -14774,7 +14774,7 @@ (define_insn_and_split "*ashl<dwi>3_doubleword_mask_1"
>
>  (define_insn "ashl<mode>3_doubleword"
>    [(set (match_operand:DWI 0 "register_operand" "=&r,&r")
> -       (ashift:DWI (match_operand:DWI 1 "reg_or_pm1_operand" "0n,r")
> +       (ashift:DWI (match_operand:DWI 1 "reg_or_pm1_operand" "0Wc,r")
>                     (match_operand:QI 2 "nonmemory_operand" "<S>c,<S>c")))
>     (clobber (reg:CC FLAGS_REG))]
>    ""
> diff --git a/gcc/testsuite/gcc.target/i386/pr116096.c b/gcc/testsuite/gcc.target/i386/pr116096.c
> new file mode 100644
> index 00000000000..5ef39805f58
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr116096.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile { target int128 } } */
> +/* { dg-options "-O2 -flive-range-shrinkage -fno-peephole2 -mstackrealign -Wno-psabi" } */
> +
> +typedef char U __attribute__((vector_size (32)));
> +typedef unsigned V __attribute__((vector_size (32)));
> +typedef __int128 W __attribute__((vector_size (32)));
> +U g;
> +
> +W baz ();
> +
> +static inline U
> +bar (V x, W y)
> +{
> +  y = y | y << (W) x;
> +  return (U)y;
> +}
> +
> +void
> +foo (W w)
> +{
> +  g = g <<
> +    bar ((V){baz ()[1], 3, 3, 5, 7},
> +        (W){w[0], ~(int) 2623676210}) >>
> +    bar ((V){baz ()[1]},
> +        (W){-w[0], ~(int) 2623676210});
> +}
> --
> 2.31.1
>
diff mbox series

Patch

diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
index 7508d7a58bd..154cbccd09e 100644
--- a/gcc/config/i386/constraints.md
+++ b/gcc/config/i386/constraints.md
@@ -254,6 +254,12 @@  (define_constraint "Wb"
   (and (match_code "const_int")
        (match_test "IN_RANGE (ival, 0, 7)")))
 
+(define_constraint "Wc"
+  "Integer constant -1 or 1."
+  (and (match_code "const_int")
+       (ior (match_test "op == constm1_rtx")
+	    (match_test "op == const1_rtx"))))
+
 (define_constraint "Ww"
   "Integer constant in the range 0 @dots{} 15, for 16-bit shifts."
   (and (match_code "const_int")
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 6207036a2a0..79d5de5b46a 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -14774,7 +14774,7 @@  (define_insn_and_split "*ashl<dwi>3_doubleword_mask_1"
 
 (define_insn "ashl<mode>3_doubleword"
   [(set (match_operand:DWI 0 "register_operand" "=&r,&r")
-	(ashift:DWI (match_operand:DWI 1 "reg_or_pm1_operand" "0n,r")
+	(ashift:DWI (match_operand:DWI 1 "reg_or_pm1_operand" "0Wc,r")
 		    (match_operand:QI 2 "nonmemory_operand" "<S>c,<S>c")))
    (clobber (reg:CC FLAGS_REG))]
   ""
diff --git a/gcc/testsuite/gcc.target/i386/pr116096.c b/gcc/testsuite/gcc.target/i386/pr116096.c
new file mode 100644
index 00000000000..5ef39805f58
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr116096.c
@@ -0,0 +1,26 @@ 
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2 -flive-range-shrinkage -fno-peephole2 -mstackrealign -Wno-psabi" } */
+
+typedef char U __attribute__((vector_size (32)));
+typedef unsigned V __attribute__((vector_size (32)));
+typedef __int128 W __attribute__((vector_size (32)));
+U g;
+
+W baz ();
+
+static inline U
+bar (V x, W y)
+{
+  y = y | y << (W) x;
+  return (U)y;
+}
+
+void
+foo (W w)
+{
+  g = g <<
+    bar ((V){baz ()[1], 3, 3, 5, 7},
+	 (W){w[0], ~(int) 2623676210}) >>
+    bar ((V){baz ()[1]},
+	 (W){-w[0], ~(int) 2623676210});
+}