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
>

Reply via email to