| 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 | expand |
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
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.
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); +}
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.