| Summary: | [EFL][GTK] Stop compiling with fno-omit-frame-pointer, -fno-tree-dce | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||
| Component: | New Bugs | Assignee: | 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
Zan Dobersek
2015-01-19 01:49:00 PST
Created attachment 244883 [details]
Patch
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. Great news, let me check it on EFL. The tested versions of GCC were: - 4.7.4 - 4.8.4 - 4.9.2 (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? 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. (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. Awesome, thanks for helping out! In the future if problems arise in this area, we would ideally disable optimizations only for the affected architectures. Created attachment 244898 [details]
Patch
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. (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. 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. (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. Can we remove the -Wno-error=uninitialized workaround too from here? http://trac.webkit.org/browser/trunk/Source/cmake/WebKitHelpers.cmake?rev=172894#L31 (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. Zan, could you land it in the near future? (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. (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. Already landed in http://trac.webkit.org/changeset/179110 |