[Bug target/105624] New: [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[Bug target/105624] New: [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[asolokha at gmx dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

            Bug ID: 105624
           Summary: [13 Regression] ICE in final_scan_insn_1, at
                    final.cc:2861 (error: could not split insn)
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: asolokha at gmx dot com
  Target Milestone: ---
            Target: x86_64-pc-linux-gnu

gcc 13.0.0 20220515 snapshot (g:18547874ee205d830acb31f1e3c1c89fc7725c14) ICEs
when compiling the following testcase w/ -march=k8 -O1:

union U {
  double d;
  unsigned long long int i;
};

double
fabs (double x)
{
  union U u;

  u.d = x;
  u.i &= ~0ULL >> 1;

  return u.d;
}

% x86_64-pc-linux-gnu-gcc-13.0.0 -march=k8 -O1 -c ifnwa4ea.c
ifnwa4ea.c: In function 'fabs':
ifnwa4ea.c:15:1: error: could not split insn
   15 | }
      | ^
(insn 8 17 18 2 (parallel [
            (set (reg:DI 0 ax [91])
                (and:DI (reg:DI 0 ax [91])
                    (reg:DI 20 xmm0 [89])))
            (clobber (reg:CC 17 flags))
        ]) "ifnwa4ea.c":14:11 530 {*anddi_1_btr}
     (expr_list:REG_DEAD (reg:DI 20 xmm0 [89])
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))
during RTL pass: final
ifnwa4ea.c:15:1: internal compiler error: in final_scan_insn_1, at
final.cc:2861
0x73a629 _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220515/work/gcc-13-20220515/gcc/rtl-error.cc:108
0x6c15f7 final_scan_insn_1
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220515/work/gcc-13-20220515/gcc/final.cc:2861
0xb6e51b final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220515/work/gcc-13-20220515/gcc/final.cc:2940
0xb6e775 final_1
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220515/work/gcc-13-20220515/gcc/final.cc:1997
0xb6f351 rest_of_handle_final
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220515/work/gcc-13-20220515/gcc/final.cc:4285
0xb6f351 execute
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220515/work/gcc-13-20220515/gcc/final.cc:4363

[Bug target/105624] [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0

[Bug target/105624] [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

--- Comment #1 from Uroš Bizjak <ubizjak at gmail dot com> ---
Ho-hum - this was my patch that removed constraint from const_int predicates.
We are talking about:

(define_insn_and_split "*anddi_1_btr"
  [(set (match_operand:DI 0 "nonimmediate_operand" "=rm")
        (and:DI
         (match_operand:DI 1 "nonimmediate_operand" "%0")
         (match_operand:DI 2 "const_int_operand")))
   (clobber (reg:CC FLAGS_REG))]

Apparently, the assumption that const_int predicates do not need cosntraints
does not hold.

[Bug target/105624] [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |ubizjak at gmail dot com
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2022-05-17

--- Comment #2 from Uroš Bizjak <ubizjak at gmail dot com> ---
I'll revert the offending patch.

[Bug target/105624] [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:

https://gcc.gnu.org/g:61a6923e8917b6efcf349515a00a485dfd5b42e4

commit r13-563-g61a6923e8917b6efcf349515a00a485dfd5b42e4
Author: Uros Bizjak <ubizjak@gmail.com>
Date:   Tue May 17 17:22:26 2022 +0200

    i386: Fix ICE in final_scan_insn_1 [PR105624]

    Apparently const_int_operand and other const*_operand predicates
    do need constraints.  Revert the offending patch that caused ICE.

    2022-05-17  Uroš Bizjak  <ubizjak@gmail.com>

    gcc/ChangeLog:

            PR target/105624
            Revert:

            * config/i386/i386.md: Remove constraints when used with
            const_int_operand, const0_operand, const_1_operand,
constm1_operand,
            const8_operand, const128_operand, const248_operand,
const123_operand,
            const2367_operand, const1248_operand, const359_operand,
            const_4_or_8_to_11_operand, const48_operand, const_0_to_1_operand,
            const_0_to_3_operand, const_0_to_4_operand, const_0_to_5_operand,
            const_0_to_7_operand, const_0_to_15_operand, const_0_to_31_operand,
            const_0_to_63_operand, const_0_to_127_operand,
const_0_to_255_operand,
            const_0_to_255_mul_8_operand, const_1_to_31_operand,
            const_1_to_63_operand, const_2_to_3_operand, const_4_to_5_operand,
            const_4_to_7_operand, const_6_to_7_operand, const_8_to_9_operand,
            const_8_to_11_operand, const_8_to_15_operand,
const_10_to_11_operand,
            const_12_to_13_operand, const_12_to_15_operand,
const_14_to_15_operand,
            const_16_to_19_operand, const_16_to_31_operand,
const_20_to_23_operand,
            const_24_to_27_operand and const_28_to_31_operand.
            * config/i386/mmx.md: Ditto.
            * config/i386/sse.md: Ditto.
            * config/i386/subst.md: Ditto.
            * config/i386/sync.md: Ditto.

    gcc/testsuite/ChangeLog:

            PR target/105624
            * gcc.target/i386/pr105624.c: New test.

[Bug target/105624] [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #4 from Uroš Bizjak <ubizjak at gmail dot com> ---
Fixed by revert.

[Bug target/105624] [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu.org

--- Comment #5 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
FWIW, I think the problem is specific to operands that are
commutative with a non-constant operand.  For example,
suppose the pre-RA instruction had a pseudo register R matching
a register_operand and a constant C matching a const_int_operand.
If R does not get allocated, and so gets replaced by a stack slot M,
the % would allow the RA to try mapping C to the register_operand
and M to the const_int_operand.  Without a constraint on the latter,
the M mapping would seem to be valid, and reloading C into a register
might seem less costly than reloading M into a register.

The intent of the patch seemed good otherwise (and a nice clean-up).
I don't think the whole thing needed to be reverted.

[Bug target/105624] [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

--- Comment #6 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to rsandifo@gcc.gnu.org from comment #5)
> FWIW, I think the problem is specific to operands that are
> commutative with a non-constant operand.  For example,
> suppose the pre-RA instruction had a pseudo register R matching
> a register_operand and a constant C matching a const_int_operand.
> If R does not get allocated, and so gets replaced by a stack slot M,
> the % would allow the RA to try mapping C to the register_operand
> and M to the const_int_operand.  Without a constraint on the latter,
> the M mapping would seem to be valid, and reloading C into a register
> might seem less costly than reloading M into a register.
> 
> The intent of the patch seemed good otherwise (and a nice clean-up).
> I don't think the whole thing needed to be reverted.

I was afraid I don't understood the reason of the failure well, although it
happened very rarely (actually, no failures were detected during the build or
testsuite run). The patch obviously triggered some inconsistency in the
infrastructure, so without some assurances, I took the safe way and reverted
everything.

I would gladly revert the revert. The reload is just doing unnecessary work
when multiple constraints are the same; all necessary information could be
retreived from the predicate.

[Bug target/105624] [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

--- Comment #7 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Uroš Bizjak from comment #6)
> I was afraid I don't understood the reason of the failure well, although it
> happened very rarely (actually, no failures were detected during the build
> or testsuite run). The patch obviously triggered some inconsistency in the
> infrastructure, so without some assurances, I took the safe way and reverted
> everything.
But like I say, I think it's due to the % in that particular instruction.
When % is used on operand N, the constraints for operands N and N+1
have to be tight enough to support both the predicate on operand N
and the predicate on operand N+1.  So for:

(define_insn_and_split "*anddi_1_btr"
  [(set (match_operand:DI 0 "nonimmediate_operand" "=rm")
        (and:DI
         (match_operand:DI 1 "nonimmediate_operand" "%0")
         (match_operand:DI 2 "const_int_operand")))
   (clobber (reg:CC FLAGS_REG))]

The constraints on operand 2 are effectively matching
nonimmediate_operand|const_int_operand rather than just
const_int_operand.

I think it would work to keep the constraints for
const_int_operands that are in a % pair and drop them
elsewhere.  (So a partial reapplication, rather than a
full reapplication.)

[Bug target/105624] [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

--- Comment #8 from Uroš Bizjak <ubizjak at gmail dot com> ---
> I think it would work to keep the constraints for
> const_int_operands that are in a % pair and drop them
> elsewhere.  (So a partial reapplication, rather than a
> full reapplication.)

OK, let's throw the patch to the wall the second time and see if it sticks this
time ;)

[Bug target/105624] [13 Regression] ICE in final_scan_insn_1, at final.cc:2861 (error: could not split insn)

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105624

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:

https://gcc.gnu.org/g:3595231d9f5aec301422b152809b1322bdb525fe

commit r13-854-g3595231d9f5aec301422b152809b1322bdb525fe
Author: Uros Bizjak <ubizjak@gmail.com>
Date:   Mon May 30 21:38:16 2022 +0200

    i386: Remove constraints when used with constant integer predicates, take 2

    const_int_operand and other const*_operand predicates do not need
    constraints when the constraint is inherited from the range of
    constant integer predicate.  Remove the constraint in case all
    alternatives use the same inherited constraint.

    However, when there are operands, commitative with a non-constant
    operand, the operand effectively matches e.g.
    nonimmediate_operand|const_int_operand rather than just
    const_int_operand.  We should keep the constraint for
    const_int_operand that are in a % pair. See PR 105624.

    2022-05-30  Uroš Bizjak  <ubizjak@gmail.com>

    gcc/ChangeLog:

            * config/i386/i386.md: Remove constraints when used with
            const_int_operand, const0_operand, const_1_operand,
constm1_operand,
            const8_operand, const128_operand, const248_operand,
const123_operand,
            const2367_operand, const1248_operand, const359_operand,
            const_4_or_8_to_11_operand, const48_operand, const_0_to_1_operand,
            const_0_to_3_operand, const_0_to_4_operand, const_0_to_5_operand,
            const_0_to_7_operand, const_0_to_15_operand, const_0_to_31_operand,
            const_0_to_63_operand, const_0_to_127_operand,
const_0_to_255_operand,
            const_0_to_255_mul_8_operand, const_1_to_31_operand,
            const_1_to_63_operand, const_2_to_3_operand, const_4_to_5_operand,
            const_4_to_7_operand, const_6_to_7_operand, const_8_to_9_operand,
            const_8_to_11_operand, const_8_to_15_operand,
const_10_to_11_operand,
            const_12_to_13_operand, const_12_to_15_operand,
const_14_to_15_operand,
            const_16_to_19_operand, const_16_to_31_operand,
const_20_to_23_operand,
            const_24_to_27_operand and const_28_to_31_operand.
            * config/i386/mmx.md: Ditto.
            * config/i386/sse.md: Ditto.
            * config/i386/subst.md: Ditto.
            * config/i386/sync.md: Ditto.

    gcc/testsuite/ChangeLog:

            * gcc.target/i386/pr105624.c: New test.