Compilation of 64bit Wine with GCC 5.3 fails when using both stack realignment (-mincoming-stack-boundary=3) and optimization (-O1 or higher): --- ../../../wine/dlls/advapi32/crypt_md4.c:134:1: internal compiler error: in choose_baseaddr, at config/i386/i386.c:10412 --- which refers to this assert in choose_baseaddr: --- gcc_assert (base_reg != NULL); --- This worked fine in some pre-5.3 dev builds (as confirmed when testing bug 66697 fix). git bisect identifies r230176 as the cause of the regression: --- commit 6ca53fc513f4efdca56af52439ffc5a49b9e6e21 Author: ebotcazou <ebotcazou@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Wed Nov 11 14:56:17 2015 +0000 PR target/67265 * ira.c (ira_setup_eliminable_regset): Do not necessarily create the frame pointer for stack checking if non-call exceptions aren't used. * config/i386/i386.c (ix86_finalize_stack_realign_flags): Likewise. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gcc-5-branch@230176 138bc75d-0d04-0410-961f-82ee72b054a4 ---
Can you provide a preprocessed testcase and the exact options you used?
Created attachment 37221 [details] testcase This testcase throws the error when compiled with: gcc -c -O2 -mincoming-stack-boundary=3 crypt_md4.i
Created attachment 37222 [details] testcase2 The previous testcase fails only with -O2 or higher; this one fails with -O1: gcc -c -O1 -mincoming-stack-boundary=3 chain.i
> This worked fine in some pre-5.3 dev builds (as confirmed when testing bug > 66697 fix). git bisect identifies r230176 as the cause of the regression: > > --- > commit 6ca53fc513f4efdca56af52439ffc5a49b9e6e21 > Author: ebotcazou <ebotcazou@138bc75d-0d04-0410-961f-82ee72b054a4> > Date: Wed Nov 11 14:56:17 2015 +0000 > > PR target/67265 > * ira.c (ira_setup_eliminable_regset): Do not necessarily create the > frame pointer for stack checking if non-call exceptions aren't used. > * config/i386/i386.c (ix86_finalize_stack_realign_flags): Likewise. Really sure? This change is a no-op unless you use -fstack-check.
> This worked fine in some pre-5.3 dev builds (as confirmed when testing bug > 66697 fix). git bisect identifies r230176 as the cause of the regression: > > --- > commit 6ca53fc513f4efdca56af52439ffc5a49b9e6e21 > Author: ebotcazou <ebotcazou@138bc75d-0d04-0410-961f-82ee72b054a4> > Date: Wed Nov 11 14:56:17 2015 +0000 > > PR target/67265 > * ira.c (ira_setup_eliminable_regset): Do not necessarily create the > frame pointer for stack checking if non-call exceptions aren't used. > * config/i386/i386.c (ix86_finalize_stack_realign_flags): Likewise. As expected, reverting the patch doesn't change anything on the 5 branch, so I'd suggest either filling a bug report for 'git bisect' or double checking its result next time.
Remove categorization based on incorrect diagnostic.
(In reply to Eric Botcazou from comment #5) > As expected, reverting the patch doesn't change anything on the 5 branch, so > I'd suggest either filling a bug report for 'git bisect' or double checking > its result next time. Sorry, I indeed made a mistake - r230176 makes compilation fail with a different error than the one I get with 5.3 release. However, after rerunning git bisect and manually checking these revisions I can confirm that: - r230165 is the last good revision in the 5 branch; - r230176 is the first one that fails, but with a different error (internal compiler error: in ix86_adjust_stack_and_probe, at config/i386/i386.c:11064); - r230247 is the first one that fails with the error in choose_baseaddr; - reverting r230176 from 5.3.0 release fixes the problem (i.e. both testcases pass and Wine compiles and runs correctly) Also, passing -v to the testcases shows that these additional options are included by default, presumably from some distro (Arch) configuration: -mtune=generic -march=x86-64 -fPIE -fstack-check=specific -fstack-protector-strong
> Sorry, I indeed made a mistake - r230176 makes compilation fail with a > different error than the one I get with 5.3 release. However, after > rerunning git bisect and manually checking these revisions I can confirm > that: > > - r230165 is the last good revision in the 5 branch; > - r230176 is the first one that fails, but with a different error (internal > compiler error: in ix86_adjust_stack_and_probe, at config/i386/i386.c:11064); > - r230247 is the first one that fails with the error in choose_baseaddr; > - reverting r230176 from 5.3.0 release fixes the problem (i.e. both > testcases pass and Wine compiles and runs correctly) I see, then please always post the full set of options. r230176 & r230247 only free the frame pointer register for the register allocator with -fstack-check (this was done at the request of the Arch folks IIRC) so you can work around the failure by forcing it to be reserved again (-fno-omit-frame-pointer). I gather that this didn't work with 5.2, did it? If so, then the alignment stuff needs more work.
Yes, forced stack alignment on x86_64 was introduced after 5.2, in r228728 for pr66697. Also, I can confirm that adding -fno-omit-frame-pointer allows Wine to compile normally.
Created attachment 37231 [details] Reduced testcase To be compiled with -O2 -mincoming-stack-boundary=3
> Yes, forced stack alignment on x86_64 was introduced after 5.2, in r228728 > for pr66697. Also, I can confirm that adding -fno-omit-frame-pointer allows > Wine to compile normally. I see, thanks. Uros, would you mind having a look? I don't know this stuff.
(In reply to Eric Botcazou from comment #11) > > Yes, forced stack alignment on x86_64 was introduced after 5.2, in r228728 > > for pr66697. Also, I can confirm that adding -fno-omit-frame-pointer allows > > Wine to compile normally. > > I see, thanks. Uros, would you mind having a look? I don't know this stuff. Ugh ... prologue and epilogue generation is not exactly the most pleasant part of the compiler. There are many different fragile code paths, so no wonder that with specialized options something breaks in this area. Looking into it.
(In reply to Uroš Bizjak from comment #12) > Looking into it. At the beginning of ix86_expand_epilogue, we have: m->fs.sp_valid = (!frame_pointer_needed || (crtl->sp_is_unchanging && !stack_realign_fp)); so, we can say in a similar way at the end of the ix86_expand_prologue: m->fs.sp_valid = !frame_pointer_needed; Justas, can you please test the following patch: --cut here-- diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index c6c66c7..9c3fa70 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -13065,6 +13065,8 @@ ix86_expand_prologue (void) m->fs.fp_valid = true; } + m->fs.sp_valid = !frame_pointer_needed; + if (!int_registers_saved) ix86_emit_save_regs_using_mov (frame.reg_save_offset); if (!sse_registers_saved) --cut here--
(In reply to Uroš Bizjak from comment #13) > Justas, can you please test the following patch Thank you, it indeed solves the problem.
(In reply to Justas L from comment #14) > (In reply to Uroš Bizjak from comment #13) > > Justas, can you please test the following patch > > Thank you, it indeed solves the problem. Please also do some runtime tests, I don't have the Wine source here ...
(In reply to Uroš Bizjak from comment #15) > Please also do some runtime tests, I don't have the Wine source here ... Wine compiles and runs with no apparent issues.
Author: uros Date: Wed Jan 6 20:19:04 2016 New Revision: 232111 URL: https://gcc.gnu.org/viewcvs?rev=232111&root=gcc&view=rev Log: PR target/69140 * config/i386/i386.c (ix86_expand_prologue): Declare fs.sp_valid depending on frame_pointer_needed before remaining integer and SSE registers are saved. testsuite/ChangeLog: PR target/69140 * gcc.target/i386/pr69140.c: New test Added: trunk/gcc/testsuite/gcc.target/i386/pr69140.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.c trunk/gcc/testsuite/ChangeLog
Ouch, we can't just make %rsp valid. I will revert previous patch and enable frame-pointer for realigned MS_ABI functions instead (if frame-pointer generation is not needed, then it will be disabled in ix86_finalize_stack_realign_flags anyway). --cut here-- Index: gcc/config/i386/i386.c =================================================================== --- gcc/config/i386/i386.c (revision 232131) +++ gcc/config/i386/i386.c (working copy) @@ -10903,6 +10903,10 @@ if (TARGET_64BIT_MS_ABI && get_frame_size () > SEH_MAX_FRAME_SIZE) return true; + /* SSE saves require frame-pointer when stack is misaligned. */ + if (TARGET_64BIT_MS_ABI && ix86_incoming_stack_boundary < 128) + return true; + /* In ix86_option_override_internal, TARGET_OMIT_LEAF_FRAME_POINTER turns off the frame pointer by default. Turn it back on now if we've not got a leaf function. */ --cut here--
Author: uros Date: Thu Jan 7 19:06:37 2016 New Revision: 232140 URL: https://gcc.gnu.org/viewcvs?rev=232140&root=gcc&view=rev Log: 2016-01-07 Uros Bizjak <ubizjak@gmail.com> PR target/69140 * config/i386/i386.c (ix86_frame_pointer_required): Enable frame pointer for TARGET_64BIT_MS_ABI when stack is misaligned. 2016-01-07 Uros Bizjak <ubizjak@gmail.com> Revert 2016-01-06 Uros Bizjak <ubizjak@gmail.com> PR target/69140 * config/i386/i386.c (ix86_expand_prologue): Declare fs.sp_valid depending on frame_pointer_needed before remaining integer and SSE registers are saved. Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.c
Author: uros Date: Mon Jan 18 16:19:53 2016 New Revision: 232528 URL: https://gcc.gnu.org/viewcvs?rev=232528&root=gcc&view=rev Log: Backport from mainline 2016-01-07 Uros Bizjak <ubizjak@gmail.com> PR target/69140 * config/i386/i386.c (ix86_frame_pointer_required): Enable frame pointer for TARGET_64BIT_MS_ABI when stack is misaligned. testsuite/ChangeLog: Backport from mainline 2016-01-06 Uros Bizjak <ubizjak@gmail.com> PR target/69140 * gcc.target/i386/pr69140.c: New test Added: branches/gcc-5-branch/gcc/testsuite/gcc.target/i386/pr69140.c Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/config/i386/i386.c branches/gcc-5-branch/gcc/testsuite/ChangeLog
Fixed.