Bug 140609

Summary: [EFL][GTK] Stop compiling with fno-omit-frame-pointer, -fno-tree-dce
Product: WebKit Reporter: Zan Dobersek <zan>
Component: New BugsAssignee: Zan Dobersek <zan>
Status: RESOLVED FIXED    
Severity: Normal CC: akiss, ggaren, gyuyoung.kim, msaboff, ossy, svillar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 142042    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch ossy: review+

Zan Dobersek
Reported 2015-01-19 01:49:00 PST
[EFL][GTK] Stop compiling with -fno-tree-dce
Attachments
Patch (2.99 KB, patch)
2015-01-19 01:52 PST, Zan Dobersek
no flags
Patch (3.04 KB, patch)
2015-01-19 05:19 PST, Zan Dobersek
ossy: review+
Zan Dobersek
Comment 1 2015-01-19 01:52:53 PST
Zan Dobersek
Comment 2 2015-01-19 01:55:34 PST
The main implication is that we insist on using the latest releases from the last three stable series of the GCC compiler. I tested all three, and there were no problems. -fno-omit-frame-pointer could perhaps be removed as well, since without it there are still no problems. I'd like to check it on ARM before that, though. (-fno-tree-dce was specific to x86-64.) The origin of all this is bug #127777.
Csaba Osztrogonác
Comment 3 2015-01-19 01:57:42 PST
Great news, let me check it on EFL.
Zan Dobersek
Comment 4 2015-01-19 02:26:02 PST
The tested versions of GCC were: - 4.7.4 - 4.8.4 - 4.9.2
Csaba Osztrogonác
Comment 5 2015-01-19 02:29:18 PST
(In reply to comment #2) > -fno-omit-frame-pointer could perhaps be removed as well, since without it > there are still no problems. I'd like to check it on ARM before that, > though. (-fno-tree-dce was specific to x86-64.) > > The origin of all this is bug #127777. Can we really remove -fno-omit-frame-pointer? I thought JSC still relies on it. ( https://bugs.webkit.org/show_bug.cgi?id=127777#c15 ) Was something changed in JSC related to this issue?
Csaba Osztrogonác
Comment 6 2015-01-19 03:09:21 PST
I tested EFL on Ubuntu 14.04 X86_64 with the stock GCC 4.8.2, all tests pass after removing -fno-omit-frame-pointer and -fno-tree-dce . ARM and Thumb2 testing are still in progress.
Csaba Osztrogonác
Comment 7 2015-01-19 04:30:27 PST
(In reply to comment #6) > ARM and Thumb2 testing are still in progress. Tests are finished, all tests passed on ARM and Thumb2, (except the flakey timeouts on Thumb2, it wasn't changed) we can remove -fno-omit-frame-pointer and -fno-tree-dce too.
Zan Dobersek
Comment 8 2015-01-19 05:12:38 PST
Awesome, thanks for helping out! In the future if problems arise in this area, we would ideally disable optimizations only for the affected architectures.
Zan Dobersek
Comment 9 2015-01-19 05:19:00 PST
Csaba Osztrogonác
Comment 10 2015-01-19 05:37:39 PST
Comment on attachment 244898 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244898&action=review r=me > Source/cmake/OptionsEfl.cmake:263 > # Push of rbp is needed after JSC JIT uses CStack. Please remove this comment before landing.
Sergio Villar Senin
Comment 11 2015-01-19 05:41:38 PST
(In reply to comment #2) > The main implication is that we insist on using the latest releases from the > last three stable series of the GCC compiler. I tested all three, and there > were no problems. > > -fno-omit-frame-pointer could perhaps be removed as well, since without it > there are still no problems. I'd like to check it on ARM before that, > though. (-fno-tree-dce was specific to x86-64.) This is important enough to be sent to the list IMO.
Csaba Osztrogonác
Comment 12 2015-01-19 06:55:59 PST
I played a little bit with bisecting which revisions fixed the issue. Without -fno-omit-frame-pointer and -fno-tree-dce options, there were ~ 1800-2000 failures previously, but https://trac.webkit.org/changeset/173282 fixed most of the failures, and https://trac.webkit.org/changeset/173298 fixed the remaining 78 failures on x86_64. I think these changes fixed the issues on ARM and Thumb2 too, but I haven't checked yet, only the ToT revision. It would be better to reference to this changesets in the changelog instead of unlikely GCC changes/fixed.
Csaba Osztrogonác
Comment 13 2015-01-19 10:16:17 PST
(In reply to comment #12) > I played a little bit with bisecting which revisions fixed the issue. > > Without -fno-omit-frame-pointer and -fno-tree-dce options, there were > ~ 1800-2000 failures previously, but https://trac.webkit.org/changeset/173282 > fixed most of the failures, and https://trac.webkit.org/changeset/173298 > fixed > the remaining 78 failures on x86_64. I think these changes fixed the issues > on ARM and Thumb2 too, but I haven't checked yet, only the ToT revision. > > It would be better to reference to this changesets in the changelog > instead of unlikely GCC changes/fixed. I validated, r173282 fixed most of the failures on ARM and Thumb2 too, and r173298 fixed the remaining failures.
Csaba Osztrogonác
Comment 14 2015-01-22 01:42:50 PST
Can we remove the -Wno-error=uninitialized workaround too from here? http://trac.webkit.org/browser/trunk/Source/cmake/WebKitHelpers.cmake?rev=172894#L31
Csaba Osztrogonác
Comment 15 2015-01-22 02:19:13 PST
(In reply to comment #14) > Can we remove the -Wno-error=uninitialized workaround too from here? > > http://trac.webkit.org/browser/trunk/Source/cmake/WebKitHelpers. > cmake?rev=172894#L31 I checked, we can remove -Wno-error=uninitialized and -Wno-error=literal-suffix too.
Csaba Osztrogonác
Comment 16 2015-01-23 08:49:03 PST
Zan, could you land it in the near future?
Zan Dobersek
Comment 17 2015-01-26 01:36:59 PST
(In reply to comment #15) > (In reply to comment #14) > > Can we remove the -Wno-error=uninitialized workaround too from here? > > > > http://trac.webkit.org/browser/trunk/Source/cmake/WebKitHelpers. > > cmake?rev=172894#L31 > > I checked, we can remove -Wno-error=uninitialized and > -Wno-error=literal-suffix too. In a separate bug, please.
Csaba Osztrogonác
Comment 18 2015-01-26 01:44:35 PST
(In reply to comment #17) > (In reply to comment #15) > > (In reply to comment #14) > > > Can we remove the -Wno-error=uninitialized workaround too from here? > > > > > > http://trac.webkit.org/browser/trunk/Source/cmake/WebKitHelpers. > > > cmake?rev=172894#L31 > > > > I checked, we can remove -Wno-error=uninitialized and > > -Wno-error=literal-suffix too. > > In a separate bug, please. It's ok for me to do it in a separated bug.
Csaba Osztrogonác
Comment 19 2015-02-26 04:58:28 PST
Note You need to log in before you can comment on or make changes to this bug.