Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1979373002:
Build libjpeg_turbo in PIC mode for ia32 and x64 builds. (Closed)
|
Created:
4 years, 7 months ago by kjellander_chromium Modified:
4 years, 7 months ago CC:
chromium-reviews, magjed_chromium Base URL:
https://chromium.googlesource.com/chromium/deps/libjpeg_turbo.git@master Target Ref:
refs/heads/master Project:
chromium_deps Visibility:
Public. |
DescriptionBuild libjpeg_turbo in PIC mode for ia32 and x64 builds.
Corresponds to the changes for GN in
https://codereview.chromium.org/1954903002/
This fixes the 'shared library text segment is not shareable' error
observed on Android x86 Builder (dbg) bot.
BUG=610145
TBR=fbarchard@chromium.org
Committed: https://chromium.googlesource.com/chromium/deps/libjpeg_turbo/+/7260e4d8b8e1e40b17f03fafdf1cd83296900f76
Patch Set 1 #
Total comments: 2
Patch Set 2 : Added for x64 as well #Messages
Total messages: 16 (7 generated)
kjellander@chromium.org changed reviewers: + fbarchard@chromium.org
Frank, do you know this code? I don't even know what PIC does, but this solves the compilation error for WebRTC. We need the GYP build to keep working, and I assume libyuv will face the same problem?
fbarchard@google.com changed reviewers: + fbarchard@google.com
-fpic means position independent code. it mainly impacts code that refers to static global variables, including strings. PIC tends to produce less efficient code, especially for 32 bit x86 where the program counter isnt easily accessible, so to find the address of a variable a code sequence does a call and pop to get the PC, then copy the variable or address somewhere, with up to 9 instructions and 3 registers used, and may cause a stall. This issue came up when libjpeg_turbo was bumped to version 1.4. 1.4 includes aarch64 neon support, so its great to have, but x86 has this fpic issue. The solution chromium came up with is similar to this, but I think included 64 bit, so I'd like to check the patch they did before we check this in.
On 2016/05/16 21:49:09, fbarchard1 wrote: > -fpic means position independent code. > it mainly impacts code that refers to static global variables, including > strings. > > PIC tends to produce less efficient code, especially for 32 bit x86 where the > program counter isnt easily accessible, so to find the address of a variable a > code sequence does a call and pop to get the PC, then copy the variable or > address somewhere, with up to 9 instructions and 3 registers used, and may cause > a stall. > > This issue came up when libjpeg_turbo was bumped to version 1.4. 1.4 includes > aarch64 neon support, so its great to have, but x86 has this fpic issue. > > The solution chromium came up with is similar to this, but I think included 64 > bit, so I'd like to check the patch they did before we check this in. Yes, Chromium's https://codereview.chromium.org/1954903002/ changed this for both 32 and 64-bit, but the only bot that breaks is 32-bit so that's why I only changed this. Do we still want to change it for both?
+CC magjed
https://codereview.chromium.org/1979373002/diff/1/libjpeg.gyp File libjpeg.gyp (right): https://codereview.chromium.org/1979373002/diff/1/libjpeg.gyp#newcode272 libjpeg.gyp:272: '-D__x86_64__', suggest add '-DPIC', for x64 as well.
PTAL @ PS#2. https://codereview.chromium.org/1979373002/diff/1/libjpeg.gyp File libjpeg.gyp (right): https://codereview.chromium.org/1979373002/diff/1/libjpeg.gyp#newcode272 libjpeg.gyp:272: '-D__x86_64__', On 2016/05/18 00:04:14, fbarchard1 wrote: > suggest add > '-DPIC', > for x64 as well. Done.
Description was changed from ========== Build libjpeg_turbo in PIC mode for x86. Corresponds to the x86 change for GN in https://codereview.chromium.org/1954903002/ This fixes the 'shared library text segment is not shareable' error observed on Android x86 Builder (dbg) bot. BUG=610145 ========== to ========== Build libjpeg_turbo in PIC mode for x86. Corresponds to the x86 change for GN in https://codereview.chromium.org/1954903002/ This fixes the 'shared library text segment is not shareable' error observed on Android x86 Builder (dbg) bot. BUG=610145 ==========
Description was changed from ========== Build libjpeg_turbo in PIC mode for x86. Corresponds to the x86 change for GN in https://codereview.chromium.org/1954903002/ This fixes the 'shared library text segment is not shareable' error observed on Android x86 Builder (dbg) bot. BUG=610145 ========== to ========== Build libjpeg_turbo in PIC mode for x86. Corresponds to the x86 change for GN in https://codereview.chromium.org/1954903002/ This fixes the 'shared library text segment is not shareable' error observed on Android x86 Builder (dbg) bot. BUG=610145 TBR=fbarchard@chromium.org ==========
TBRing fbarchard since this doesn't change anything until rolled in (to Chromium/WebRTC DEPS etc).
Description was changed from ========== Build libjpeg_turbo in PIC mode for x86. Corresponds to the x86 change for GN in https://codereview.chromium.org/1954903002/ This fixes the 'shared library text segment is not shareable' error observed on Android x86 Builder (dbg) bot. BUG=610145 TBR=fbarchard@chromium.org ========== to ========== Build libjpeg_turbo in PIC mode for ia32 and x64 builds. Corresponds to the x86 change for GN in https://codereview.chromium.org/1954903002/ This fixes the 'shared library text segment is not shareable' error observed on Android x86 Builder (dbg) bot. BUG=610145 TBR=fbarchard@chromium.org ==========
Description was changed from ========== Build libjpeg_turbo in PIC mode for ia32 and x64 builds. Corresponds to the x86 change for GN in https://codereview.chromium.org/1954903002/ This fixes the 'shared library text segment is not shareable' error observed on Android x86 Builder (dbg) bot. BUG=610145 TBR=fbarchard@chromium.org ========== to ========== Build libjpeg_turbo in PIC mode for ia32 and x64 builds. Corresponds to the changes for GN in https://codereview.chromium.org/1954903002/ This fixes the 'shared library text segment is not shareable' error observed on Android x86 Builder (dbg) bot. BUG=610145 TBR=fbarchard@chromium.org ==========
Description was changed from ========== Build libjpeg_turbo in PIC mode for ia32 and x64 builds. Corresponds to the changes for GN in https://codereview.chromium.org/1954903002/ This fixes the 'shared library text segment is not shareable' error observed on Android x86 Builder (dbg) bot. BUG=610145 TBR=fbarchard@chromium.org ========== to ========== Build libjpeg_turbo in PIC mode for ia32 and x64 builds. Corresponds to the changes for GN in https://codereview.chromium.org/1954903002/ This fixes the 'shared library text segment is not shareable' error observed on Android x86 Builder (dbg) bot. BUG=610145 TBR=fbarchard@chromium.org Committed: https://chromium.googlesource.com/chromium/deps/libjpeg_turbo/+/7260e4d8b8e1e... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 7260e4d8b8e1e40b17f03fafdf1cd83296900f76 (presubmit successful).
Message was sent while issue was closed.
lgtm |
