[Bug rtl-optimization/88070] New: [8/9 Regression] ICE in create_pre_exit, at mode-switching.c:438

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug rtl-optimization/88070] New: [8/9 Regression] ICE in create_pre_exit, at mode-switching.c:438

msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88070

            Bug ID: 88070
           Summary: [8/9 Regression] ICE in create_pre_exit, at
                    mode-switching.c:438
           Product: gcc
           Version: 8.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: asolokha at gmx dot com
  Target Milestone: ---
            Target: x86_64-unknown-linux-gnu

gcc-9.0.0-alpha20181111 snapshot (r266019) ICEs when compiling the following
snippet extracted from gcc/testsuite/gcc.dg/pr63594-2.c at any optimization
level except -Os and w/ -mavx -fexpensive-optimizations -fnon-call-exceptions
-fschedule-insns -fno-dce -fno-dse:

typedef float vfloat2 __attribute__ ((__vector_size__ (2 * sizeof (float))));

__attribute__((noinline, noclone)) vfloat2
test1float2 (float c)
{
  vfloat2 v = { c, c };
  return v;
}

% x86_64-unknown-linux-gnu-gcc-9.0.0-alpha20181111 -mavx -O1
-fexpensive-optimizations -fnon-call-exceptions -fschedule-insns -fno-dce
-fno-dse -c jsud8y1a.c
during RTL pass: vzeroupper
jsud8y1a.c: In function 'test1float2':
jsud8y1a.c:8:1: internal compiler error: in create_pre_exit, at
mode-switching.c:438
    8 | }
      | ^
0x757f43 create_pre_exit
       
/var/tmp/portage/sys-devel/gcc-9.0.0_alpha20181111/work/gcc-9-20181111/gcc/mode-switching.c:438
0x757f43 optimize_mode_switching
       
/var/tmp/portage/sys-devel/gcc-9.0.0_alpha20181111/work/gcc-9-20181111/gcc/mode-switching.c:534
0x757f43 execute
       
/var/tmp/portage/sys-devel/gcc-9.0.0_alpha20181111/work/gcc-9-20181111/gcc/mode-switching.c:892
0xff0b94 rest_of_handle_insert_vzeroupper
       
/var/tmp/portage/sys-devel/gcc-9.0.0_alpha20181111/work/gcc-9-20181111/gcc/config/i386/i386.c:891
0xff0b94 execute
       
/var/tmp/portage/sys-devel/gcc-9.0.0_alpha20181111/work/gcc-9-20181111/gcc/config/i386/i386.c:2511
Reply | Threaded
Open this post in threaded view
|

[Bug target/88070] [8/9 Regression] ICE in create_pre_exit, at mode-switching.c:438

msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88070

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2018-11-18
   Target Milestone|---                         |7.4
     Ever confirmed|0                           |1

--- Comment #1 from Uroš Bizjak <ubizjak at gmail dot com> ---
This is the case of scheduler splitting return copy pair:

(insn 19 18 16 2 (set (reg:V2SF 21 xmm0)
        (mem/c:V2SF (plus:DI (reg/f:DI 7 sp)
                (const_int -72 [0xffffffffffffffb8])) [0  S8 A64]))
"pr88070.c":8 1157 {*movv2sf_internal}
     (nil))
(insn 16 19 20 2 (set (reg:V2SF 0 ax [orig:91 <retval> ] [91])
        (reg:V2SF 0 ax [89])) "pr88070.c":8 1157 {*movv2sf_internal}
     (nil))
(insn 20 16 21 2 (unspec_volatile [
            (const_int 0 [0])
        ] UNSPECV_BLOCKAGE) "pr88070.c":8 710 {blockage}
     (nil))
(insn 21 20 23 2 (use (reg:V2SF 21 xmm0)) "pr88070.c":8 -1
     (nil))

Please note how (insn 16) interferes with (insn 19)-(insn 21) pair.

I think that the assert is too restrictive for post-reload vzeroupper
insertion. There will be no end of problems, similar to the one above (mainly
due to pre-reload scheduler, or the RA itself), so I guess the following patch,
that relaxes the assert is justified:

--cut here--
Index: mode-switching.c
===================================================================
--- mode-switching.c    (revision 266250)
+++ mode-switching.c    (working copy)
@@ -431,11 +431,12 @@
              }
            while (nregs);

-           /* If we didn't see a full return value copy, verify that there
-              is a plausible reason for this.  If some, but not all of the
-              return register is likely spilled, we can expect that there
-              is a copy for the likely spilled part.  */
-           gcc_assert (!nregs
+           /* Before reload, if we didn't see a full return value copy,
+              verify that there is a plausible reason for this.  If some,
+              but not all of the return register is likely spilled, we can
+              expect that there is a copy for the likely spilled part.  */
+           gcc_assert (reload_completed
+                       || !nregs
                        || forced_late_switch
                        || short_block
                        || !(targetm.class_likely_spilled_p
--cut here--
Reply | Threaded
Open this post in threaded view
|

[Bug target/88070] [8/9 Regression] ICE in create_pre_exit, at mode-switching.c:438

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88070

Martin Liška <marxin at gcc dot gnu.org> changed:

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

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Just for the record: started with r255395.
Reply | Threaded
Open this post in threaded view
|

[Bug target/88070] [8/9 Regression] ICE in create_pre_exit, at mode-switching.c:438

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88070

--- Comment #3 from Uroš Bizjak <ubizjak at gmail dot com> ---
For reference:

The assert in create_pre_exit at mode-switching.c expects return copy
pair with nothing in between. However, the compiler starts mode
switching pass with the following sequence:

(insn 19 18 16 2 (set (reg:V2SF 21 xmm0)
        (mem/c:V2SF (plus:DI (reg/f:DI 7 sp)
                (const_int -72 [0xffffffffffffffb8])) [0  S8 A64]))
"pr88070.c":8 1157 {*movv2sf_internal}
     (nil))
(insn 16 19 20 2 (set (reg:V2SF 0 ax [orig:91 <retval> ] [91])
        (reg:V2SF 0 ax [89])) "pr88070.c":8 1157 {*movv2sf_internal}
     (nil))
(insn 20 16 21 2 (unspec_volatile [
            (const_int 0 [0])
        ] UNSPECV_BLOCKAGE) "pr88070.c":8 710 {blockage}
     (nil))
(insn 21 20 23 2 (use (reg:V2SF 21 xmm0)) "pr88070.c":8 -1
     (nil))

Please note how (insn 16) interferes with (insn 19)-(insn 21) return copy pair.

The culprit for this is the blockage instruction (insn 20), which
causes sched1 pass (pre reload scheduler) to skip marking (insn 19) as
unmovable instruction (as a dependent insn on return use insn), so the
scheduler is free to schedule (insn 16) between return copy pair (insn
19)-(insn 21).

The extra instruction is generated as a kludge in expand_function_end
to prevent instructions that may trap to be scheduled into function
epilogue. However, the same blockage is generated under exactly the
same conditions earlier in the expand_function_end. The first blockage
should prevent unwanted scheduling into the epilogue, so there is
actually no need for the second one.
Reply | Threaded
Open this post in threaded view
|

[Bug target/88070] [8/9 Regression] ICE in create_pre_exit, at mode-switching.c:438

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88070

--- Comment #4 from Uroš Bizjak <ubizjak at gmail dot com> ---
BTW: The extra blockage would crash compilation for all mode-switching
targets, also in the pre-reload mode switching; the vzeroupper
post-reload insertion just trips x86 target on a generic problem in
the middle-end.

Proposed patch:

--cut here--
Index: function.c
===================================================================
--- function.c  (revision 266278)
+++ function.c  (working copy)
@@ -5447,13 +5447,6 @@ expand_function_end (void)
   if (naked_return_label)
     emit_label (naked_return_label);

-  /* @@@ This is a kludge.  We want to ensure that instructions that
-     may trap are not moved into the epilogue by scheduling, because
-     we don't always emit unwind information for the epilogue.  */
-  if (cfun->can_throw_non_call_exceptions
-      && targetm_common.except_unwind_info (&global_options) != UI_SJLJ)
-    emit_insn (gen_blockage ());
-
   /* If stack protection is enabled for this function, check the guard.  */
   if (crtl->stack_protect_guard && targetm.stack_protect_runtime_enabled_p ())
     stack_protect_epilogue ();
--cut here--
Reply | Threaded
Open this post in threaded view
|

[Bug target/88070] [8/9 Regression] ICE in create_pre_exit, at mode-switching.c:438

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88070

--- Comment #5 from Uroš Bizjak <ubizjak at gmail dot com> ---
*** Bug 85673 has been marked as a duplicate of this bug. ***
Reply | Threaded
Open this post in threaded view
|

[Bug target/88070] [8/9 Regression] ICE in create_pre_exit, at mode-switching.c:438

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88070

--- Comment #6 from Uroš Bizjak <ubizjak at gmail dot com> ---
It looks that we need to relax mode-switching:

--cut here--
Index: mode-switching.c
===================================================================
--- mode-switching.c    (revision 266278)
+++ mode-switching.c    (working copy)
@@ -252,7 +252,21 @@ create_pre_exit (int n_entities, int *entity_map,
        if (EDGE_COUNT (EXIT_BLOCK_PTR_FOR_FN (cfun)->preds) == 1
            && NONJUMP_INSN_P ((last_insn = BB_END (src_bb)))
            && GET_CODE (PATTERN (last_insn)) == USE
-           && GET_CODE ((ret_reg = XEXP (PATTERN (last_insn), 0))) == REG)
+           && GET_CODE ((ret_reg = XEXP (PATTERN (last_insn), 0))) == REG
+
+           /* x86 targets use mode-switching infrastructure to
+              conditionally insert vzeroupper instruction at the exit
+              from the function where there is no need to switch the
+              mode before the return value copy.  The vzeroupper insertion
+              pass runs after reload, so use !reload_completed as a stand-in
+              for x86 to skip the search for the return value copy insn.
+
+              N.b.: the code below assumes that the return copy insn
+              immediately precedes its corresponding use insn.  This
+              assumption does not hold after reload, since sched1 pass
+              can schedule the return copy insn away from its
+              corresponding use insn.  */
+           && !reload_completed)
          {
            int ret_start = REGNO (ret_reg);
            int nregs = REG_NREGS (ret_reg);
--cut here--
Reply | Threaded
Open this post in threaded view
|

[Bug target/88070] [8/9 Regression] ICE in create_pre_exit, at mode-switching.c:438

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88070

--- Comment #7 from uros at gcc dot gnu.org ---
Author: uros
Date: Tue Nov 20 19:43:20 2018
New Revision: 266326

URL: https://gcc.gnu.org/viewcvs?rev=266326&root=gcc&view=rev
Log:
        PR target/88070
        * mode-switching.c (create_pre_exit): After reload, always split the
        fallthrough edge to the exit block.

testsuite/ChangeLog:

        PR target/88070
        * gcc.target/i386/pr88070.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr88070.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/mode-switching.c
    trunk/gcc/testsuite/ChangeLog
Reply | Threaded
Open this post in threaded view
|

[Bug target/88070] [8 Regression] ICE in create_pre_exit, at mode-switching.c:438

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88070

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.4                         |7.5
Reply | Threaded
Open this post in threaded view
|

[Bug target/88070] [8 Regression] ICE in create_pre_exit, at mode-switching.c:438

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88070

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.5                         |8.4