1610814 - NEON accelerated RGB unpacking code has compile error with gcc

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Andrew Osmond [:aosmond] (he/him)]

As reported by tt_1 in bug 1601622:

9:16.49 gmake[4]: Entering directory '/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ff/gfx/2d'
9:16.50 gfx/2d/SwizzleNEON.o
9:16.50 /usr/bin/armv7a-unknown-linux-gnueabihf-g++ -std=gnu++17 -o SwizzleNEON.o -c -flto -flifetime-dse=1 -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ff/dist/stl_wrappers -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ff/dist/system_wrappers -include /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DUSE_NEON -DOS_POSIX=1 -DOS_LINUX=1 -DUSE_CAIRO -DMOZ2D_HAS_MOZ_CAIRO -DMOZ_ENABLE_FREETYPE -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ff/gfx/2d -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ff/ipc/ipdl/_ipdlheaders -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ipc/chromium/src -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ipc/glue -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/skia -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/skia/skia -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ff/dist/include -I/usr/armv7a-unknown-linux-gnueabihf/usr/include/nspr -I/usr/include/nss -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ff/dist/include/nss -I/usr/armv7a-unknown-linux-gnueabihf/usr/include/pixman-1 -fPIC -DMOZILLA_CLIENT -include /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ff/mozilla-config.h -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wimplicit-fallthrough -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wno-error=multistatement-macros -Wno-error=class-memaccess -Wno-error=deprecated-copy -Wformat -Wformat-security -Wformat-overflow=2 -fno-sized-deallocation -fno-aligned-new -pipe -flifetime-dse=1 -Wno-psabi -Wno-class-memaccess -Wno-int-in-bool-context -Wno-multistatement-macros -Wno-maybe-uninitialized -Wno-deprecated-declarations -mthumb -mno-thumb-interwork -mfpu=neon -mfloat-abi=hard -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -O2 -fomit-frame-pointer -funwind-tables -Wno-error=shadow -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ff/dist/include/cairo -I/usr/armv7a-unknown-linux-gnueabihf/usr/include/freetype2 -I/usr/armv7a-unknown-linux-gnueabihf/usr/include/uuid -I/usr/armv7a-unknown-linux-gnueabihf/usr/include/freetype2 -MD -MP -MF .deps/SwizzleNEON.o.pp -fdiagnostics-color -mfpu=neon /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp
9:16.50 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp: In function 'void mozilla::gfx::UnpackRowRGB24_NEON(const uint8_t*, uint8_t*, int32_t)':
9:16.50 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp:423:51: note: use '-flax-vector-conversions' to permit conversions between vectors with differing element types or numbers of subparts
9:16.50 423 | vst1q_u16(reinterpret_cast<uint16_t*>(dst), px);
9:16.50 | ^
9:16.50 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp:423:49: error: cannot convert 'uint8x16_t' {aka '__vector(16) unsigned char'} to 'uint16x8_t' {aka '__vector(8) short unsigned int'}
9:16.50 423 | vst1q_u16(reinterpret_cast<uint16_t*>(dst), px);
9:16.50 | ^~
9:16.50 | |
9:16.50 | uint8x16_t {aka __vector(16) unsigned char}
9:16.50 In file included from /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp:9:
9:16.50 /usr/lib/gcc/armv7a-unknown-linux-gnueabihf/9.2.0/include/arm_neon.h:10984:39: note: initializing argument 2 of 'void vst1q_u16(uint16_t*, uint16x8_t)'
9:16.50 10984 | vst1q_u16 (uint16_t * __a, uint16x8_t __b)
9:16.50 | ^
9:16.50 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp: In instantiation of 'void mozilla::gfx::UnpackRowRGB24_NEON(const uint8_t*, uint8_t*, int32_t) [with bool aSwapRB = false; uint8_t = unsigned char; int32_t = int]':
9:16.50 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp:430:75: required from here
9:16.50 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp:415:30: error: cannot convert 'uint16x8_t' {aka '__vector(8) short unsigned int'} to 'uint8x16_t' {aka '__vector(16) unsigned char'} in initialization
9:16.50 415 | uint8x16_t px = vld1q_u16(reinterpret_cast<const uint16_t*>(src));
9:16.50 | ^
9:16.50 | |
9:16.50 | uint16x8_t {aka __vector(8) short unsigned int}
9:16.50 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp: In instantiation of 'void mozilla::gfx::UnpackRowRGB24_NEON(const uint8_t*, uint8_t*, int32_t) [with bool aSwapRB = true; uint8_t = unsigned char; int32_t = int]':
9:16.50 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp:431:74: required from here
9:16.50 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp:415:30: error: cannot convert 'uint16x8_t' {aka '__vector(8) short unsigned int'} to 'uint8x16_t' {aka '__vector(16) unsigned char'} in initialization
9:16.50 gmake[4]: *** [/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/config/rules.mk:738: SwizzleNEON.o] Error 1
9:16.50 gmake[4]: Leaving directory '/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/ff/gfx/2d'
9:16.50 gmake[3]: *** [/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta3/work/firefox-73.0/config/recurse.mk:74: gfx/2d/target-objects] Error 2
9:16.50 gmake[3]: *** Waiting for unfinished jobs....

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1610814 - Fix NEON compile error with gcc and RGB unpacking.

This patch makes us use the correct intrinsic for loading a uint8x16
register. It is not entirely clear why clang accepts this without
complaint but beyond the types, it should be equivalent.

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48d53f90a789eb0b9268fbe35df87cfce4901652

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

Can you confirm that this patch corrects your problem? Thanks. Sorry for the delay in looking at this. Your original report is much appreciated :).

Comment 4

[tt_1]

thank you for picking this up, it's just that there must be another one just a few lines below:

7:35.31 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta8/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp:423:49: error: cannot convert 'uint8x16_t' {aka '__vector(16) unsigned char'} to 'uint16x8_t' {aka '__vector(8) short unsigned int'}
7:35.31 423 | vst1q_u16(reinterpret_cast<uint16_t*>(dst), px);
7:35.31 | ^~
7:35.31 | |
7:35.31 | uint8x16_t {aka __vector(16) unsigned char}
7:35.31 In file included from /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-73.0_beta8/work/firefox-73.0/gfx/2d/SwizzleNEON.cpp:9:

Comment 5

[tt_1]

Attachment: build.log.gz

here's the full log, for better new lines and so on.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

Thanks. I updated the phabricator review to fix that one as well (hopefully; I'll need to wait for try to confirm since I'm not setup to do an ARM build right now).

Comment 7

[tt_1]

It's all good now :)

can you please merge this to 73.0-beta branch as well?

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3d8b08265b8
Fix NEON compile error with gcc and RGB unpacking. r=lsalzman

Comment 9

[Alexandru Michis [:malexandru]]

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

Comment 10

[tt_1]

can you please add the to the firefox-73 beta branch too?

Comment 11

[tt_1]

it seems I'm having a bad bugzilla day, I meant to say: please add the patch to the firefox-73 beta branch.

Comment 12

[tt_1]

ok, whatever, the fix will be available in 74 beta

Comment 13

[Andrew Osmond [:aosmond] (he/him)]

Sorry I missed this. It will make the next beta in a week, we are already in RC for 73.