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

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect, P3)

Description

[Georg Koppen]

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.

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1331335 Add SSE2 Flags to all of libANGLE

This updates moz.build and accompanies https://github.com/mozilla/angle/pull/12 to update the python generator

Review commit: https://reviewboard.mozilla.org/r/105624/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/105624/

Comment 2

[Jacek Caban]

Attachment: 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.

Comment 3

[Jacek Caban]

I sent that upstream:
https://chromium-review.googlesource.com/#/c/434638/

Comment 4

[Georg Koppen]

(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

[Jacek Caban]

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

Comment 6

[Georg Koppen]

Attachment: loadimage_new-3d08a8e5ed5700e98814fbe3c2be44e9.cpp

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

[Jacek Caban]

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...

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

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?

Comment 9

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

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?

Comment 10

[Kelsey Gilbert [:jgilbert]]

We require SSE2, so add the needed flag.

Comment 11

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

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

[Kelsey Gilbert [:jgilbert]]

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 13

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/105624/diff/1-2/

Comment 14

[Kelsey Gilbert [:jgilbert]]

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

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8573b1243ee
Add SSE2 Flags to all of libANGLE r=jgilbert

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e8573b1243ee

Comment 17

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

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.

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/b724283e3b31