[v2,middle,end] : Fix PR58372, internal compiler error: ix86_compute_frame_layout

Message ID CAFULd4ZO68sirbWgaw9Y8RRK6R6GqqmpyRe8JHCGpCu-Zrr_nQ@mail.gmail.com
State New
Headers show
Series
  • [v2,middle,end] : Fix PR58372, internal compiler error: ix86_compute_frame_layout
Related show

Commit Message

Uros Bizjak Nov. 1, 2018, 4:18 p.m.
Hello!

v2 of the patch hits the real problem: in pass_expand::execute
finish_eh_generation is called after expand_stack_alignment is called.
Construction of SjLj landing pads calls emit_library_call, which can
change crtl->preferred_stack_boundary value after all dependant
variables are already calculated by expand_stack_alignment.

The solution is to move the call to finish_eh_generation in front of
the call to expand_stack_alignment.

2018-11-01  Uros Bizjak  <ubizjak@gmail.com>

    PR middle-end/58372
    * cfgexpand.c (pass_expand::execute): Move the call to
    finish_eh_generation in front of the call to expand_stack_alignment.

testsuite/ChangeLog:

2018-11-01  Uros Bizjak  <ubizjak@gmail.com>

    PR middle-end/58372
    * g++.target/i386/pr58372.C: New test.

Patch was bootstrapped and regression tested on x86_64-linux-gnu
{,-m32}, all default languages plus go. Additionally, the testcase
from PR (and a couple of similar ones) were compiled for
i686-w64-mingw32 target with various combinations of
-mpreferred-stack-boundary= -mincoming-stack-boundary= -mforce-drap
and -m{no-}accumulate-outgoing-args.

OK for mainline and release branches?

Uros.

Comments

Jeff Law Nov. 4, 2018, 5:59 p.m. | #1
On 11/1/18 10:18 AM, Uros Bizjak wrote:
> Hello!

> 

> v2 of the patch hits the real problem: in pass_expand::execute

> finish_eh_generation is called after expand_stack_alignment is called.

> Construction of SjLj landing pads calls emit_library_call, which can

> change crtl->preferred_stack_boundary value after all dependant

> variables are already calculated by expand_stack_alignment.

> 

> The solution is to move the call to finish_eh_generation in front of

> the call to expand_stack_alignment.

> 

> 2018-11-01  Uros Bizjak  <ubizjak@gmail.com>

> 

>     PR middle-end/58372

>     * cfgexpand.c (pass_expand::execute): Move the call to

>     finish_eh_generation in front of the call to expand_stack_alignment.

> 

> testsuite/ChangeLog:

> 

> 2018-11-01  Uros Bizjak  <ubizjak@gmail.com>

> 

>     PR middle-end/58372

>     * g++.target/i386/pr58372.C: New test.

> 

> Patch was bootstrapped and regression tested on x86_64-linux-gnu

> {,-m32}, all default languages plus go. Additionally, the testcase

> from PR (and a couple of similar ones) were compiled for

> i686-w64-mingw32 target with various combinations of

> -mpreferred-stack-boundary= -mincoming-stack-boundary= -mforce-drap

> and -m{no-}accumulate-outgoing-args.

> 

> OK for mainline and release branches?

> 

> Uros.

> 

OK, but please add a comment indicating why the new sequencing is needed
in the code.

jeff
Uros Bizjak Nov. 4, 2018, 7:25 p.m. | #2
On Sun, Nov 4, 2018 at 6:59 PM Jeff Law <law@redhat.com> wrote:
>

> On 11/1/18 10:18 AM, Uros Bizjak wrote:

> > Hello!

> >

> > v2 of the patch hits the real problem: in pass_expand::execute

> > finish_eh_generation is called after expand_stack_alignment is called.

> > Construction of SjLj landing pads calls emit_library_call, which can

> > change crtl->preferred_stack_boundary value after all dependant

> > variables are already calculated by expand_stack_alignment.

> >

> > The solution is to move the call to finish_eh_generation in front of

> > the call to expand_stack_alignment.

> >

> > 2018-11-01  Uros Bizjak  <ubizjak@gmail.com>

> >

> >     PR middle-end/58372

> >     * cfgexpand.c (pass_expand::execute): Move the call to

> >     finish_eh_generation in front of the call to expand_stack_alignment.

> >

> > testsuite/ChangeLog:

> >

> > 2018-11-01  Uros Bizjak  <ubizjak@gmail.com>

> >

> >     PR middle-end/58372

> >     * g++.target/i386/pr58372.C: New test.

> >

> > Patch was bootstrapped and regression tested on x86_64-linux-gnu

> > {,-m32}, all default languages plus go. Additionally, the testcase

> > from PR (and a couple of similar ones) were compiled for

> > i686-w64-mingw32 target with various combinations of

> > -mpreferred-stack-boundary= -mincoming-stack-boundary= -mforce-drap

> > and -m{no-}accumulate-outgoing-args.

> >

> > OK for mainline and release branches?

> >

> > Uros.

> >

> OK, but please add a comment indicating why the new sequencing is needed

> in the code.


Thanks, committed with the following comment:

  /* Call expand_stack_alignment after finishing all
     updates to crtl->preferred_stack_boundary.  */
  expand_stack_alignment ();

I'll backport the patch to release branches after a week without
problems in the mainline.

Uros.

Patch

Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 265582)
+++ cfgexpand.c	(working copy)
@@ -6510,6 +6510,12 @@  pass_expand::execute (function *fun)
   find_many_sub_basic_blocks (blocks);
   purge_all_dead_edges ();
 
+  /* After initial rtl generation, call back to finish generating
+     exception support code.  We need to do this before cleaning up
+     the CFG as the code does not expect dead landing pads.  */
+  if (fun->eh->region_tree != NULL)
+    finish_eh_generation ();
+
   expand_stack_alignment ();
 
   /* Fixup REG_EQUIV notes in the prologue if there are tailcalls in this
@@ -6517,12 +6523,6 @@  pass_expand::execute (function *fun)
   if (crtl->tail_call_emit)
     fixup_tail_calls ();
 
-  /* After initial rtl generation, call back to finish generating
-     exception support code.  We need to do this before cleaning up
-     the CFG as the code does not expect dead landing pads.  */
-  if (fun->eh->region_tree != NULL)
-    finish_eh_generation ();
-
   /* BB subdivision may have created basic blocks that are are only reachable
      from unlikely bbs but not marked as such in the profile.  */
   if (optimize)
Index: testsuite/g++.target/i386/pr58372.C
===================================================================
--- testsuite/g++.target/i386/pr58372.C	(nonexistent)
+++ testsuite/g++.target/i386/pr58372.C	(working copy)
@@ -0,0 +1,9 @@ 
+/* PR target/58372 */
+/* { dg-do compile { target c++14 } } */
+/* { dg-options "-O2" } */
+
+__attribute__ ((__target__ ("rdrnd")))
+void f (unsigned int *b) noexcept
+{
+  __builtin_ia32_rdrand32_step (b);
+}