Closed
Bug 1378529
Opened 7 years ago
Closed 7 years ago
third_party/aom causes 'error: multiple storage classes in declaration specifiers' during MinGW compile
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: tjr, Assigned: tjr)
References
(Blocks 1 open bug)
Details
(Whiteboard: [tor])
Attachments
(1 file)
Compiling with MinGW throws the following errors: > /mingw-w64-build/firefox/third_party/aom/aom_ports/x86.h:121:1: error: multiple storage classes in declaration specifiers > static INLINE uint64_t xgetbv(void) { > ^ > /mingw-w64-build/firefox/third_party/aom/aom_ports/x86.h:169:1: error: multiple storage classes in declaration specifiers > static INLINE int x86_simd_caps(void) { > ^ > /mingw-w64-build/firefox/third_party/aom/aom_ports/x86.h:227:1: error: multiple storage classes in declaration specifiers > static INLINE unsigned int x86_readtsc(void) { > ^ > /mingw-w64-build/firefox/third_party/aom/aom_ports/x86.h:245:1: error: multiple storage classes in declaration specifiers > static INLINE uint64_t x86_readtsc64(void) { > ^ > /mingw-w64-build/firefox/third_party/aom/aom_ports/x86.h:310:1: error: multiple storage classes in declaration specifiers > static INLINE unsigned int x87_set_double_precision(void) { > ^ I was under the impression that the following were the only storage-class specifiers: - typedef - extern - static - auto - register - _Thread_local INLINE gets defined to either 'inline' or '__forceinline'. It appears[0] that mingw gives __forceinline an extern qualifier. I suspect this is not going to be a project open to upstreaming, so my inclination is to do some sort of local or global undef or redefinition for forceinline... [0] https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-git/0002-Undefine-FORCEINLINE-on-MinGW-w64-in-malloc.c.h.patch
Assignee | ||
Comment 2•7 years ago
|
||
Thanks to :rillian, I figured out a better solution seems. aom has a win64-gcc target that defines INLINE as 'inline' instead of '_forceinline'. That definition change fixes this. Unfortunately, aom does not have a win32-gcc target, so the goal is to upstream adding a win32-gcc target, then update and use that target for MinGW.
Flags: needinfo?(jacek)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8885380 -
Flags: review?(giles)
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8885380 [details] Bug 1378529 Use the MinGW libaom configuration for MinGW builds on Linux https://reviewboard.mozilla.org/r/156234/#review162598 ::: media/libaom/config/win/mingw32/aom_config.asm:28 (Diff revision 2) > .equ HAVE_FEXCEPT , 1 > .equ HAVE_PTHREAD_H , 0 > .equ HAVE_WXWIDGETS , 0 > .equ CONFIG_DEPENDENCY_TRACKING , 1 > .equ CONFIG_EXTERNAL_BUILD , 1 > -.equ CONFIG_INSTALL_DOCS , 1 > +.equ CONFIG_INSTALL_DOCS , 0 Is this spurious, maybe from importing a different revision? ::: media/libaom/generate_sources_mozbuild.sh:222 (Diff revision 2) > gen_rtcd_header linux/x64 x86_64 > gen_rtcd_header linux/ia32 x86 > gen_rtcd_header mac/x64 x86_64 > gen_rtcd_header win/x64 x86_64 > gen_rtcd_header win/ia32 x86 > +gen_rtcd_header win/mingw32 x86 Oops! This part should probably be rolled into the other import-script changes from bug 1380118. ::: media/libaom/moz.build:40 (Diff revision 2) > + CFLAGS += [ '-I%s/media/libaom/config/win/mingw32/' % TOPSRCDIR ] > + EXPORTS.aom += [ 'config/win/mingw32/aom_config.h' ] > + else: > - CFLAGS += [ '-I%s/media/libaom/config/win/ia32/' % TOPSRCDIR ] > + CFLAGS += [ '-I%s/media/libaom/config/win/ia32/' % TOPSRCDIR ] > - EXPORTS.aom += [ 'config/win/ia32/aom_config.h' ] > + EXPORTS.aom += [ 'config/win/ia32/aom_config.h' ] > + ASFLAGS += [ '-I%s/media/libaom/config/win/ia32/' % TOPSRCDIR ] Even if using the same ASFLAGS work, the two branches here should probably have separate configs to make sure they match the CFLAGS settings.
Attachment #8885380 -
Flags: review?(giles) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
I simplified the patch, can you re-review? If it looks good, I will land it once Bug 1380118 lands.
Flags: needinfo?(giles)
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8885380 [details] Bug 1378529 Use the MinGW libaom configuration for MinGW builds on Linux https://reviewboard.mozilla.org/r/156234/#review174166 Thanks for the clean-up! Still r=me with the issues addressed. ::: media/libaom/moz.build:34 (Diff revision 3) > elif CONFIG['CPU_ARCH'] == 'x86': > EXPORTS.aom += files['IA32_EXPORTS'] > SOURCES += files['IA32_SOURCES'] > USE_YASM = True > if CONFIG['OS_TARGET'] == 'WINNT': > + if CONFIG['HOST_OS_ARCH'] == 'Linux': Looking at this now, I think this branch will be triggered by linux-hosted clang-cl builds. I think you can avoid this by appending `and not CONFIG['CLANG_CL']` to the conditional here. ::: media/libaom/moz.build:35 (Diff revision 3) > EXPORTS.aom += files['IA32_EXPORTS'] > SOURCES += files['IA32_SOURCES'] > USE_YASM = True > if CONFIG['OS_TARGET'] == 'WINNT': > + if CONFIG['HOST_OS_ARCH'] == 'Linux': > + ASFLAGS += [ '-I%s/media/libaom/config/win/ia32/' % TOPSRCDIR ] Again, please use the mingw32 ASFLAGS to make sure they match the CFLAGS.
Updated•7 years ago
|
Flags: needinfo?(giles)
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8885380 [details] Bug 1378529 Use the MinGW libaom configuration for MinGW builds on Linux https://reviewboard.mozilla.org/r/156234/#review175092 ::: media/libaom/moz.build:35 (Diff revisions 3 - 4) > EXPORTS.aom += files['IA32_EXPORTS'] > SOURCES += files['IA32_SOURCES'] > USE_YASM = True > if CONFIG['OS_TARGET'] == 'WINNT': > - if CONFIG['HOST_OS_ARCH'] == 'Linux': > - ASFLAGS += [ '-I%s/media/libaom/config/win/ia32/' % TOPSRCDIR ] > + if CONFIG['CC_TYPE'] == 'gcc': > + ASFLAGS += [ '-I%s/media/libaom/config/win/mingw32/' % TOPSRCDIR ] Looks good. Thanks for fixing!
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 11•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f4f05166b2be Use the MinGW libaom configuration for MinGW builds on Linux r=rillian
Keywords: checkin-needed
![]() |
||
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f4f05166b2be
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•