Hi Hongtao,
Thanks for the review.  Good catch.  This isn't a problem now, but is a
latent issue
should the stv2 pass ever allow SUBREGs to reach this code.  The patch below
makes
the obvious change, what was previously

> -      gcc_assert (GET_MODE (*op) == vmode);
> +      if (GET_MODE (*op) != V1TImode)
> +     *op = gen_lowpart (V1TImode, *op);

is now replaced with:

> -      gcc_assert (GET_MODE (*op) == vmode);
> +      if (GET_MODE (*op) != vmode)
> +     *op = gen_lowpart (vmode, *op);

Likewise, I've updated to ChangeLog to reflect that functions have been
renamed/refactored since this patch was originally written back in 2023.

This revision 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?


2026-05-27  Roger Sayle  <[email protected]>
            Hongtao Liu  <[email protected]>

gcc/ChangeLog
        * config/i386/i386-features.cc (scalar_chain::add_insn): Don't
        call analyze_register_chain if the USE is a SUBREG.
        (scalar_chain::convert_op): Call gen_lowpart to convert
        scalar (TImode) SUBREGs to vector (V1TImode) SUBREGs.
        (convertible_comparison_p): We can now handle all general_operands
        of *cmp<dwi>_doubleword.
        (timode_remove_non_convertible_regs): We only need to check TImode
        uses that aren't TImode SUBREGs of registers in other modes.

gcc/testsuite/ChangeLog
        * gcc.target/i386/sse4_1-ptest-7.c: New test case.


Thanks,
Roger
--

> -----Original Message-----
> From: Liu, Hongtao <[email protected]>
> Sent: 18 May 2026 03:23
> To: Roger Sayle <[email protected]>; 'GCC Patches' <gcc-
> [email protected]>
> Cc: 'Hongtao Liu' <[email protected]>; 'Uros Bizjak' <[email protected]>
> Subject: RE: [x86_64 PING] Handle SUBREG conversions in TImode STV (for
> ptest).
> 
> @@ -1163,7 +1164,8 @@ scalar_chain::convert_op (rtx *op, rtx_insn *insn)
>    else
>      {
>        gcc_assert (SUBREG_P (*op));
> -      gcc_assert (GET_MODE (*op) == vmode);
> +      if (GET_MODE (*op) != V1TImode)
> +     *op = gen_lowpart (V1TImode, *op);
> 
> Convert_op is shared by timode_scalar_chain (vmode=V1TImode) and
> general_scalar_chain, hardcoded V1TImode may generate invalid rtx?
> The vmode could be V2DImode(for ia32), and src now is V1TImode?
> 
> > -----Original Message-----
> > From: Roger Sayle <[email protected]>
> > Sent: Saturday, May 16, 2026 4:27 PM
> > To: 'GCC Patches' <[email protected]>
> > Cc: Liu, Hongtao <[email protected]>; 'Hongtao Liu'
> > <[email protected]>; 'Uros Bizjak' <[email protected]>
> > Subject: [x86_64 PING] Handle SUBREG conversions in TImode STV (for
ptest).
> >
> >
> > I'd like to ping/resubmit this patch from June 2023:
> > https://gcc.gnu.org/pipermail/gcc-patches/2023-June/622706.html
> >
> > Re-tested against mainline with no failures.  The only change is to
> > add -mno- stackrealign to the new test case, to keep STV happy on some
> > operating systems.  Ok for mainline?
> >
> >
> > 2026-05-16  Roger Sayle  <[email protected]>
> >
> > gcc/ChangeLog
> >         * config/i386/i386-features.cc (scalar_chain::add_insn): Don't
> >         call analyze_register_chain if the USE is a SUBREG.
> >         (timode_scalar_chain::convert_op): Call gen_lowpart to convert
> >         TImode SUBREGs to V1TImode SUBREGs.
> >         (convertible_comparison_p): We can now handle all
general_operands
> >         of *cmp<dwi>_doubleword.
> >         (timode_remove_non_convertible_regs): We only need to check
TImode
> >         uses that aren't TImode SUBREGs of registers in other modes.
> >
> > gcc/testsuite/ChangeLog
> >         * gcc.target/i386/sse4_1-ptest-7.c: New test case.
> >
> >
> > Thanks in advance,
> > Roger
> > --

diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc
index ce5f0e9c178..4f5e9c84bf0 100644
--- a/gcc/config/i386/i386-features.cc
+++ b/gcc/config/i386/i386-features.cc
@@ -523,7 +523,8 @@ scalar_chain::add_insn (bitmap candidates, unsigned int 
insn_uid,
       }
 
   for (ref = DF_INSN_UID_USES (insn_uid); ref; ref = DF_REF_NEXT_LOC (ref))
-    if (!DF_REF_REG_MEM_P (ref))
+    if (DF_REF_TYPE (ref) == DF_REF_REG_USE
+       && !SUBREG_P (DF_REF_REG (ref)))
       if (!analyze_register_chain (candidates, ref, disallowed))
        return false;
 
@@ -1163,7 +1164,8 @@ scalar_chain::convert_op (rtx *op, rtx_insn *insn)
   else
     {
       gcc_assert (SUBREG_P (*op));
-      gcc_assert (GET_MODE (*op) == vmode);
+      if (GET_MODE (*op) != vmode)
+       *op = gen_lowpart (vmode, *op);
     }
 }
 
@@ -2274,12 +2276,8 @@ convertible_comparison_p (rtx_insn *insn, enum 
machine_mode mode)
   rtx op2 = XEXP (src, 1);
 
   /* *cmp<dwi>_doubleword.  */
-  if ((CONST_SCALAR_INT_P (op1)
-       || ((REG_P (op1) || MEM_P (op1))
-          && GET_MODE (op1) == mode))
-      && (CONST_SCALAR_INT_P (op2)
-         || ((REG_P (op2) || MEM_P (op2))
-             && GET_MODE (op2) == mode)))
+  if (general_operand (op1, mode)
+      && general_operand (op2, mode))
     return true;
 
   /* *testti_doubleword.  */
@@ -2623,8 +2621,9 @@ timode_remove_non_convertible_regs (bitmap candidates)
                                               DF_REF_REGNO (ref));
 
        FOR_EACH_INSN_USE (ref, insn)
-         if (!DF_REF_REG_MEM_P (ref)
-             && GET_MODE (DF_REF_REG (ref)) == TImode)
+         if (DF_REF_TYPE (ref) == DF_REF_REG_USE
+             && GET_MODE (DF_REF_REG (ref)) == TImode
+             && !SUBREG_P (DF_REF_REG (ref)))
            timode_check_non_convertible_regs (candidates, regs,
                                               DF_REF_REGNO (ref));
       }
diff --git a/gcc/testsuite/gcc.target/i386/sse4_1-ptest-7.c 
b/gcc/testsuite/gcc.target/i386/sse4_1-ptest-7.c
new file mode 100644
index 00000000000..2b7c049d155
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/sse4_1-ptest-7.c
@@ -0,0 +1,22 @@
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2 -msse4.1 -mno-stackrealign" } */
+
+typedef long long __m128i __attribute__ ((__vector_size__ (16)));
+
+int foo (__m128i x, __m128i y)
+{
+  return (__int128)x == (__int128)y;
+}
+
+int bar (__m128i x, __m128i y)
+{
+  return (__int128)(x^y) == 0;
+}
+
+int baz (__m128i x, __m128i y)
+{
+  return (__int128)(x==y) == ~0;
+}
+
+/* { dg-final { scan-assembler-times "ptest\[ \\t\]+%" 3 } } */
+/* { dg-final { scan-assembler-not "%\[er\]sp" } } */

Reply via email to