Compiling SSE2 code in libANGLE with mingw-w64 fails with: error: inlining failed in call to always_inline

RESOLVED FIXED in Firefox -esr52

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gk, Assigned: tjr)

Tracking

(Blocks 1 bug)

51 Branch
mozilla55
x86
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 fixed, firefox55 fixed)

Details

(Whiteboard: [tor][gfx-noted])

Attachments

(3 attachments)

Reporter

Description

2 years ago
The compilation fails with:

/home/firefox/win52/toolchain/mingw-w64/bin/i686-w64-mingw32-g++ -std=gnu++11 -mwindows -o Unified_cpp_angle_src_libANGLE6.o -c -I/home/firefox/win52/mozilla-aurora/obj-mingw/dist/stl_wrappers  -DNDEBUG=1 -DTRIMMED=1 -D_CRT_SECURE_NO_DEPRECATE -D_HAS_EXCEPTIONS=0 -D_SECURE_SCL=0 -DANGLE_ENABLE_D3D9 -DANGLE_SKIP_DXGI_1_2_CHECK -DANGLE_ENABLE_D3D11 -DANGLE_COMPILE_OPTIMIZATION_LEVEL=D3DCOMPILE_OPTIMIZATION_LEVEL1 -DANGLE_NO_EXCEPTIONS -DGL_APICALL= -DGL_GLEXT_PROTOTYPES= -DEGLAPI= -DLIBANGLE_IMPLEMENTATION=1 -DANGLE_ENABLE_HLSL=1 -DANGLE_ENABLE_GLSL=1 -DANGLE_ENABLE_ESSL=1 -DANGLE_ENABLE_KEYEDMUTEX=1 -DANGLE_DEFAULT_D3D11=0 -I/home/firefox/win52/mozilla-aurora/gfx/angle/src/libANGLE -I/home/firefox/win52/mozilla-aurora/obj-mingw/gfx/angle/src/libANGLE -I/home/firefox/win52/mozilla-aurora/gfx/angle/include -I/home/firefox/win52/mozilla-aurora/gfx/angle/src -I/home/firefox/win52/mozilla-aurora/gfx/angle/src/common/third_party/numerics -I/home/firefox/win52/mozilla-aurora/gfx/angle/src/third_party/khronos -I/home/firefox/win52/mozilla-aurora/obj-mingw/dist/include  -I/home/firefox/win52/mozilla-aurora/obj-mingw/dist/include/nspr -I/home/firefox/win52/mozilla-aurora/obj-mingw/dist/include/nss         -DMOZILLA_CLIENT -include /home/firefox/win52/mozilla-aurora/obj-mingw/mozilla-config.h -MD -MP -MF .deps/Unified_cpp_angle_src_libANGLE6.o.pp  -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++14-compat -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-format -fno-exceptions -fno-strict-aliasing -mms-bitfields -mstackrealign -fno-keep-inline-dllexport -fno-rtti -fno-exceptions -fno-math-errno -pipe  -g -O -fomit-frame-pointer -Wno-attributes -Wno-shadow -Wno-sign-compare -Wno-unknown-pragmas -Wno-unreachable-code -Wno-shadow-compatible-local -Wno-shadow-local  /home/firefox/win52/mozilla-aurora/obj-mingw/gfx/angle/src/libANGLE/Unified_cpp_angle_src_libANGLE6.cpp
In file included from /home/firefox/win52/mozilla-aurora/obj-mingw/gfx/angle/src/libANGLE/Unified_cpp_angle_src_libANGLE6.cpp:65:0:
/home/firefox/win52/mozilla-aurora/gfx/angle/src/image_util/loadimage.cpp: In function ‘void angle::LoadA8ToRGBA8(size_t, size_t, size_t, const uint8_t*, size_t, size_t, uint8_t*, size_t, size_t)’:
/home/firefox/win52/mozilla-aurora/gfx/angle/src/image_util/loadimage.cpp:31:46: warning: SSE vector return without SSE enabled changes the ABI [-Wpsabi]
         __m128i zeroWide = _mm_setzero_si128();
                                              ^
In file included from /home/firefox/win52/toolchain/mingw-w64/lib/gcc/i686-w64-mingw32/5.4.0/include/xmmintrin.h:1249:0,
                 from /home/firefox/win52/toolchain/mingw-w64/lib/gcc/i686-w64-mingw32/5.4.0/include/x86intrin.h:31,
                 from /home/firefox/win52/toolchain/mingw-w64/i686-w64-mingw32/include/intrin.h:73,
                 from /home/firefox/win52/mozilla-aurora/gfx/angle/src/common/platform.h:48,
                 from /home/firefox/win52/mozilla-aurora/gfx/angle/src/common/angleutils.h:12,
                 from /home/firefox/win52/mozilla-aurora/gfx/angle/src/compiler/translator/IntermNode.h:24,
                 from /home/firefox/win52/mozilla-aurora/gfx/angle/src/compiler/translator/VersionGLSL.h:10,
                 from /home/firefox/win52/mozilla-aurora/gfx/angle/src/compiler/translator/VersionGLSL.cpp:7,
                 from /home/firefox/win52/mozilla-aurora/obj-mingw/gfx/angle/src/libANGLE/Unified_cpp_angle_src_libANGLE6.cpp:2:
/home/firefox/win52/toolchain/mingw-w64/lib/gcc/i686-w64-mingw32/5.4.0/include/emmintrin.h:753:1: error: inlining failed in call to always_inline ‘__m128i _mm_setzero_si128()’: target specific option mismatch
 _mm_setzero_si128 (void)

"-msse2" is missing. While loadimageSSE2.cpp is gone in the update to chromium/2845 and thus the need for adding "-msse2" while compiling it, the SSE2 code in libANGLE is still there.
Reporter

Updated

2 years ago
Summary: Compiling SSE2 code with mingw-w64 fails with: error: inlining failed in call to always_inline target specific option mismatch → Compiling SSE2 code in libANGLE with mingw-w64 fails with: error: inlining failed in call to always_inline
Priority: -- → P3
Whiteboard: [tor] → [tor][gfx-noted]
Assignee

Updated

2 years ago
Assignee: nobody → tom
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Blocks: 1330608

Comment 2

2 years ago
The proposed patch will drop support for non-SSE2 platforms, since the whole file will potentially use SSE2, without checking for its support. It may be not too bad, I'm not sure.

GCC has a pragma that allows controlling which functions support SSE. The attached patch uses them to fix the problem.
Reporter

Comment 4

2 years ago
(In reply to Jacek Caban from comment #2)
> Created attachment 8828300 [details] [diff] [review]
> Fix with pragmas
> 
> The proposed patch will drop support for non-SSE2 platforms, since the whole
> file will potentially use SSE2, without checking for its support. It may be
> not too bad, I'm not sure.
> 
> GCC has a pragma that allows controlling which functions support SSE. The
> attached patch uses them to fix the problem.

FWIW: I can't get libANGLE compiled with that fix on a 32bit Ubuntu Precise and a toolchain using the latest mingw-w64 + GCC 5.1.0.

It breaks with:

/home/firefox/win52/mozilla-central/gfx/angle/src/image_util/loadimage.cpp: In function ‘void angle::LoadA8ToRGBA8(size_t, size_t, size_t, const uint8_t*, size_t, size_t, uint8_t*, size_t, size_t)’:
/home/firefox/win52/mozilla-central/gfx/angle/src/image_util/loadimage.cpp:152:129: error: calling ‘void angle::LoadA8ToRGBA8SSE2(size_t, size_t, size_t, const uint8_t*, size_t, size_t, uint8_t*, size_t, size_t)’ with SSE caling convention without SSE/SSE2 enabled
         LoadA8ToRGBA8SSE2(width, height, depth, input, inputRowPitch, inputDepthPitch, output, outputRowPitch, outputDepthPitch);
                                                                                                                                 ^

So, hm.

Comment 5

2 years ago
It seems that I attached a patch with broken #ifdefs. Could you please try the patch I sent to ANGLE?
Reporter

Comment 6

2 years ago
Hm, the problem does not go away with the patch you uploaded there (I attached the loadimage.cpp file I used to this bug in case I messed things up on my side).

Fixing the problem in the moz.build file by adding '-msse2' to the CXXFLAGS where needed works for me, though.

Comment 7

2 years ago
I'm on GCC 6.2, so maybe that makes a difference. Anyway, if pragmas don't fix it reliably, then I think we should use your fix. Possibly broken ANGLE in mingw build on non-SSE2-capable hardware is not the worst thing...
Assignee

Comment 8

2 years ago
I don't believe non-SSE2 is an issue - #1284902 indicates that they will not be able to get past FF48. 

Adding Vlad to advise on and review - which patch would you like and is it acceptable?
Flags: needinfo?(vladimir)
Assignee

Comment 9

2 years ago
Hey Jeff, do you think you could give your thoughts about which patch you'd prefer to land (mine which requires SSE2 or Jacek's which does not) and if there's anything you'd like changed?
Flags: needinfo?(vladimir) → needinfo?(jgilbert)
We require SSE2, so add the needed flag.
Flags: needinfo?(jgilbert)
Assignee

Updated

2 years ago
Attachment #8828112 - Flags: review?(jgilbert)
Assignee

Comment 11

2 years ago
Okay, thanks, that's mine - I flagged you for review. If you like it and r+ it I can get it merged in, but you would need to merge https://github.com/mozilla/angle/pull/12/files

Comment 12

2 years ago
mozreview-review
Comment on attachment 8828112 [details]
Bug 1331335 Add SSE2 Flags to all of libANGLE

https://reviewboard.mozilla.org/r/105624/#review137040

::: gfx/angle/src/libANGLE/moz.build:297
(Diff revision 1)
> +    '../image_util/loadimage.cpp',
>      'Display.cpp',
>      'renderer/d3d/DisplayD3D.cpp',
>      'renderer/d3d/HLSLCompiler.cpp',
>  ]
> +SOURCES['../image_util/loadimage.cpp'].flags += CONFIG['SSE2_FLAGS']

This is going to get forgotten when we update. Mark all of ANGlE as SSE2 instead.
Comment hidden (mozreview-request)

Comment 14

2 years ago
mozreview-review
Comment on attachment 8828112 [details]
Bug 1331335 Add SSE2 Flags to all of libANGLE

https://reviewboard.mozilla.org/r/105624/#review140720
Attachment #8828112 - Flags: review?(jgilbert) → review+
Assignee

Updated

2 years ago
Keywords: checkin-needed

Comment 15

2 years ago
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e8573b1243ee
Add SSE2 Flags to all of libANGLE r=jgilbert
Keywords: checkin-needed

Comment 16

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e8573b1243ee
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee

Comment 17

2 years ago
Comment on attachment 8828112 [details]
Bug 1331335 Add SSE2 Flags to all of libANGLE

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: This will fix the build for MinGW, resulting in one less patch Tor Browser will have to carry themselves.

User impact if declined: Tor will have to add another custom patch on top of our branch.

Fix Landed on Version: 55

Risk to taking this patch (and alternatives if risky): Low risk? The build might break?

String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8828112 - Flags: approval-mozilla-esr52?
Comment on attachment 8828112 [details]
Bug 1331335 Add SSE2 Flags to all of libANGLE

Seems very low risk, user impact seems justified, let's uplift to ESR52
Attachment #8828112 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.