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" } } */