Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2845433002:
Remove enable_frame_pointers and enable frame-pointers under ChromeOS. (Closed)
DescriptionRemove enable_frame_pointers and enable frame-pointers under ChromeOS.
As suggested by thakis@, this avoids adding yet another build argument
which can potentially be set inconsistently with other things.
Since ChromeOS needs frame-pointers enabled in x64 builds, to support
CWP, and temporarily in ARM/Thumb builds, to avoid CPU errata, we now
always enable them under ChromeOS. The ChromeOS toolchain already set
frame-pointers always on so this restores the pre-M59 behaviour.
BUG=BUG=710131, 706654, 711784
Review-Url: https://codereview.chromium.org/2845433002
Cr-Commit-Position: refs/heads/master@{#467386}
Committed: https://chromium.googlesource.com/chromium/src/+/8f2e1bde3eb4c84fcfe13962a2f6b48223526f9b
Patch Set 1 #
Messages
Total messages: 18 (10 generated)
wez@chromium.org changed reviewers: + dianders@chromium.org, dpranke@chromium.org, erikchen@chromium.org, manojgupta@chromium.org
dpranke: plz OWNERS review. erikchen, manojgupta, dianders: FYI
The CQ bit was checked by wez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by wez@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1493230878091650, "parent_rev":
"55cde5fb732692857b3535076e5ac9e08188033e", "commit_rev":
"8f2e1bde3eb4c84fcfe13962a2f6b48223526f9b"}
Message was sent while issue was closed.
Description was changed from ========== Remove enable_frame_pointers and enable frame-pointers under ChromeOS. As suggested by thakis@, this avoids adding yet another build argument which can potentially be set inconsistently with other things. Since ChromeOS needs frame-pointers enabled in x64 builds, to support CWP, and temporarily in ARM/Thumb builds, to avoid CPU errata, we now always enable them under ChromeOS. The ChromeOS toolchain already set frame-pointers always on so this restores the pre-M59 behaviour. BUG=BUG=710131, 706654, 711784 ========== to ========== Remove enable_frame_pointers and enable frame-pointers under ChromeOS. As suggested by thakis@, this avoids adding yet another build argument which can potentially be set inconsistently with other things. Since ChromeOS needs frame-pointers enabled in x64 builds, to support CWP, and temporarily in ARM/Thumb builds, to avoid CPU errata, we now always enable them under ChromeOS. The ChromeOS toolchain already set frame-pointers always on so this restores the pre-M59 behaviour. BUG=BUG=710131, 706654, 711784 Review-Url: https://codereview.chromium.org/2845433002 Cr-Commit-Position: refs/heads/master@{#467386} Committed: https://chromium.googlesource.com/chromium/src/+/8f2e1bde3eb4c84fcfe13962a2f6... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/8f2e1bde3eb4c84fcfe13962a2f6...
Message was sent while issue was closed.
On 2017/04/26 18:28:46, commit-bot: I haz the power wrote: > Committed patchset #1 (id:1) as > https://chromium.googlesource.com/chromium/src/+/8f2e1bde3eb4c84fcfe13962a2f6... Have you measured the perf impact of this change?
Message was sent while issue was closed.
Description was changed from ========== Remove enable_frame_pointers and enable frame-pointers under ChromeOS. As suggested by thakis@, this avoids adding yet another build argument which can potentially be set inconsistently with other things. Since ChromeOS needs frame-pointers enabled in x64 builds, to support CWP, and temporarily in ARM/Thumb builds, to avoid CPU errata, we now always enable them under ChromeOS. The ChromeOS toolchain already set frame-pointers always on so this restores the pre-M59 behaviour. BUG=BUG=710131, 706654, 711784 Review-Url: https://codereview.chromium.org/2845433002 Cr-Commit-Position: refs/heads/master@{#467386} Committed: https://chromium.googlesource.com/chromium/src/+/8f2e1bde3eb4c84fcfe13962a2f6... ========== to ========== Remove enable_frame_pointers and enable frame-pointers under ChromeOS. As suggested by thakis@, this avoids adding yet another build argument which can potentially be set inconsistently with other things. Since ChromeOS needs frame-pointers enabled in x64 builds, to support CWP, and temporarily in ARM/Thumb builds, to avoid CPU errata, we now always enable them under ChromeOS. The ChromeOS toolchain already set frame-pointers always on so this restores the pre-M59 behaviour. BUG=BUG=710131, 706654, 711784 Review-Url: https://codereview.chromium.org/2845433002 Cr-Commit-Position: refs/heads/master@{#467386} Committed: https://chromium.googlesource.com/chromium/src/+/8f2e1bde3eb4c84fcfe13962a2f6... ==========
Message was sent while issue was closed.
marcheu@chromium.org changed reviewers: + ihf@google.com, marcheu@chromium.org
Message was sent while issue was closed.
The ChromeOS tool-chain forces -fno-omit-frame-pointers across all platforms at present - the fact that we changed the Chrome build to override that was what revealed the ARM A12/A17 errata - so this CL is actually just restoring things to the way they were in M58. So the short answer is: No, I have not checked perf impact. On 26 April 2017 at 13:45, <marcheu@chromium.org> wrote: > On 2017/04/26 18:28:46, commit-bot: I haz the power wrote: > > Committed patchset #1 (id:1) as > > > https://chromium.googlesource.com/chromium/src/+/ > 8f2e1bde3eb4c84fcfe13962a2f6b48223526f9b > > Have you measured the perf impact of this change? > > https://codereview.chromium.org/2845433002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
