On Thu, Dec 4, 2025 at 11:52 AM Jakub Jelinek <[email protected]> wrote:
>
> Hi!
>
> The following testcase ICEs on x86_64, because the crc_rev_optab expander
> assumes the last operand will be a CONST_INT. That assumption comes from
> it being created with
> rtx polynomial;
> if (TREE_CODE (rhs3) != INTEGER_CST)
> {
> error ("third argument to %<crc%> builtins must be a constant");
> polynomial = const0_rtx;
> }
> else
> polynomial = convert_to_mode (TYPE_MODE (result_type), expand_normal
> (rhs3), 0);
> and so it doesn't bother adding a predicate for it.
> Except that maybe_legitimize_operands which expand_insn calls has:
> This avoids duplicate rtl and ensures that tied operands
> remain tied.
>
> This search is linear, but NOPS is bounded at compile time
> to a small number (current a single digit). */
> unsigned int j = 0;
> for (; j < i; ++j)
> if (can_reuse_operands_p (icode, opno + j, opno + i, &ops[j], &ops[i])
> && rtx_equal_p (orig_values[j], orig_values[i])
> && ops[j].value
> && insn_operand_matches (icode, opno + i, ops[j].value))
> {
> ops[i].value = copy_rtx (ops[j].value);
> break;
> }
> in it, so if one of the earlier operands has equal original value to the
> polynomial argument, but has a predicate like register_operand or
> nonimmediate_operand, the earlier iteration forced that value into a pseudo
> and when the last operand doesn't have a predicate, this happily reuses that
> pseudo as the last operand. And then it either with RTL checking fails on
> INTVAL use on that operand, or without rtl checking ICEs during expansion of
> the insn e.g. using table lookup.
>
> The following patch fixes it by using const_int_operand predicate for it.
> That is what loongarch and riscv backends use for it too. Aarch64 doesn't
> and I'll send a fix for that once tested on aarch64-linux.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2025-12-04 Jakub Jelinek <[email protected]>
>
> PR target/122991
> * config/i386/i386.md (crc_rev<SWI124:mode>si4): Use const_int_operand
> predicate for the last input argument.
>
> * gcc.dg/pr122991.c: New test.
OK.
Thanks,
Uros.
>
> --- gcc/config/i386/i386.md.jj 2025-11-18 19:56:17.249637175 +0100
> +++ gcc/config/i386/i386.md 2025-12-03 23:13:51.152339465 +0100
> @@ -29711,7 +29711,7 @@ (define_expand "crc_rev<SWI124:mode>si4"
> [(match_operand:SI 0 "register_operand")
> (match_operand:SI 1 "register_operand")
> (match_operand:SWI124 2 "nonimmediate_operand")
> - (match_operand:SI 3)]
> + (match_operand:SI 3 "const_int_operand")]
> "TARGET_CRC32"
> {
> /* crc32 uses iSCSI polynomial */
> --- gcc/testsuite/gcc.dg/pr122991.c.jj 2025-12-04 10:30:26.382263713 +0100
> +++ gcc/testsuite/gcc.dg/pr122991.c 2025-12-04 10:31:01.927649913 +0100
> @@ -0,0 +1,28 @@
> +/* PR target/122991 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-additional-options "-msse4" { target i?86-*-* x86_64-*-* } } */
> +
> +int
> +foo ()
> +{
> + return __builtin_rev_crc32_data32 (0, 0, 0);
> +}
> +
> +int
> +bar ()
> +{
> + return __builtin_rev_crc32_data32 (-1U, -1U, -1U);
> +}
> +
> +int
> +baz ()
> +{
> + return __builtin_crc32_data32 (0, 0, 0);
> +}
> +
> +int
> +qux ()
> +{
> + return __builtin_crc32_data32 (-1U, -1U, -1U);
> +}
>
> Jakub
>