Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(190)

Issue 705053002: Enable zlib SIMD optimisations on Mac OS X builds (Closed)

Created:
6 years, 1 month ago by robert.bradford
Modified:
6 years, 1 month ago
Reviewers:
agl
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Enable zlib SIMD optimisations on Mac OS X builds With ninja builds generated from gyp on a Mac it is necessary to specify the cflags to use as EXTRA_CFLAGS in an xcode_settings block in the gyp file. With gn this is not necessary and led to confusion resulting in Mac builds not being enabled in the original CL. As all Intel based Mac systems have SSE2 the compiler does not require "-msse2" as it can always use that. This change also initialised the global variables, as a result the variable is moved from being a common symbol to being in the BSS section of the library. This is necessary as unfortunately the linker on Mac cannot handle common symbols with static libraries. blink layout test inspector/layers/layer-canvas-log.html may start failing on Mac with this change (See BUG below) TEST=Build on Mac OS X and net_unittests --gtest_filter=*Compressed* run and uses optimised code paths BUG=430557 Committed: https://crrev.com/01686c1a7b233fcb8ab16b41ad3ffe58aef09cd1 Cr-Commit-Position: refs/heads/master@{#304089}

Patch Set 1 #

Patch Set 2 : Keep disabled on ios; initialise symbols so we're in the BSS #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -5 lines) Patch
M third_party/zlib/simd_stub.c View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/zlib/x86.c View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/zlib/zlib.gyp View 1 2 chunks +5 lines, -3 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
robert.bradford
Hi Adam, I was able to identify and fix the build issue on Mac OS ...
6 years, 1 month ago (2014-11-07 12:29:04 UTC) #2
agl
lgtm Sorry, I missed the email notification for this one.
6 years, 1 month ago (2014-11-13 20:29:08 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/705053002/20001
6 years, 1 month ago (2014-11-13 20:29:50 UTC) #5
commit-bot: I haz the power
Committed patchset #2 (id:20001)
6 years, 1 month ago (2014-11-13 21:39:07 UTC) #6
commit-bot: I haz the power
6 years, 1 month ago (2014-11-13 21:40:08 UTC) #7
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/01686c1a7b233fcb8ab16b41ad3ffe58aef09cd1
Cr-Commit-Position: refs/heads/master@{#304089}

Powered by Google App Engine
This is Rietveld 408576698