1677690 - build fails after introducing SIMD support on aarch64

Reporter

Description

•

4 years ago

After introducing SIMD support for aarch64 thru bug 1609381 we are seeing the following build failure on Linux/aarch64 (Fedora 31 and 32 with gcc9 and gcc10). This is with a default config.

...
gmake[4]: Vstupuje se do adresáře „/home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu/js/src/wasm“
js/src/wasm/Unified_cpp_js_src_wasm0.o
/usr/bin/g++ -std=gnu++17 -o Unified_cpp_js_src_wasm0.o -c  -I/home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu/dist/system_wrappers -include /home/sharkcz/projects/firefox/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DWASM_SUPPORTS_HUGE_MEMORY -DJS_CACHEIR_SPEW -DJS_STRUCTURED_SPEW -DJS_HAS_CTYPES -DFFI_BUILDING -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -I/home/sharkcz/projects/firefox/js/src/wasm -I/home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu/js/src/wasm -I/home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu/js/src -I/home/sharkcz/projects/firefox/js/src -I/home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu/dist/include -I/home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu/dist/include/nspr -fPIC -DMOZILLA_CLIENT -include /home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu/js/src/js-confdefs.h -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++2a-compat -Wduplicated-cond -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wno-multistatement-macros -Wno-error=class-memaccess -Wno-error=deprecated-copy -Wformat -Wformat-overflow=2 -Werror=implicit-function-declaration -Wno-psabi -fno-sized-deallocation -fno-aligned-new -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O3 -fno-omit-frame-pointer -funwind-tables -fno-strict-aliasing -Werror=format -Wno-shadow -Wno-attributes  -MD -MP -MF .deps/Unified_cpp_js_src_wasm0.o.pp  -fdiagnostics-color  Unified_cpp_js_src_wasm0.cpp
cc1plus: warning: ‘-Werror=’ argument ‘-Werror=implicit-function-declaration’ is not valid for C++
In file included from /home/sharkcz/projects/firefox/js/src/vm/Activation.h:25,
                 from /home/sharkcz/projects/firefox/js/src/vm/JSContext.h:28,
                 from /home/sharkcz/projects/firefox/js/src/vm/GlobalObject.h:33,
                 from /home/sharkcz/projects/firefox/js/src/frontend/CompilationInfo.h:28,
                 from /home/sharkcz/projects/firefox/js/src/frontend/Parser.h:184,
                 from /home/sharkcz/projects/firefox/js/src/wasm/AsmJS.cpp:38,
                 from Unified_cpp_js_src_wasm0.cpp:2:
/home/sharkcz/projects/firefox/js/src/vm/Stack.h: In instantiation of ‘class js::detail::FixedArgsBase<js::NO_CONSTRUCT, 0>’:
/home/sharkcz/projects/firefox/js/src/vm/Stack.h:933:7:   required from ‘class js::FixedInvokeArgs<0>’
/home/sharkcz/projects/firefox/js/src/vm/Interpreter.h:90:29:   required from here
/home/sharkcz/projects/firefox/js/src/vm/Stack.h:890:19: warning: comparison is always true due to limited range of data type [-Wtype-limits]
  890 |   static_assert(N <= ARGS_LENGTH_MAX, "o/~ too many args o/~");
      |                 ~~^~~~~~~~~~~~~~~~~~
In file included from Unified_cpp_js_src_wasm0.cpp:11:
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:661:13: error: explicit specialization in non-namespace scope ‘class js::wasm::BaseRegAlloc’
  661 |   template <>
      |             ^
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:662:8: error: template-id ‘hasFPU<js::jit::MIRType::Simd128>’ in declaration of primary template
  662 |   bool hasFPU<MIRType::Simd128>() {
      |        ^~~~~~~~~~~~~~~~~~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:747:17: error: too many template-parameter-lists
  747 |   FloatRegister allocFPU() {
      |                 ^~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:752:13: error: explicit specialization in non-namespace scope ‘class js::wasm::BaseRegAlloc’
  752 |   template <>
      |             ^
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:753:17: error: expected ‘;’ at end of member declaration
  753 |   FloatRegister allocFPU<MIRType::Simd128>() {
      |                 ^~~~~~~~
      |                         ;
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:753:17: error: ‘js::jit::FloatRegister js::wasm::BaseRegAlloc::allocFPU’ conflicts with a previous declaration
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:738:8: note: previous declaration ‘void js::wasm::BaseRegAlloc::allocFPU(js::jit::FloatRegister)’
  738 |   void allocFPU(FloatRegister r) {
      |        ^~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:753:25: error: expected unqualified-id before ‘<’ token
  753 |   FloatRegister allocFPU<MIRType::Simd128>() {
      |                         ^
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp: In member function ‘js::wasm::RegF32 js::wasm::BaseRegAlloc::needF32()’:
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:937:27: error: invalid use of non-static member function ‘void js::wasm::BaseRegAlloc::allocFPU(js::jit::FloatRegister)’
  937 |     return RegF32(allocFPU<MIRType::Float32>());
      |                   ~~~~~~~~^~~~~~~~~~~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:738:8: note: declared here
  738 |   void allocFPU(FloatRegister r) {
      |        ^~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:937:18: error: expected primary-expression before ‘(’ token
  937 |     return RegF32(allocFPU<MIRType::Float32>());
      |                  ^
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:937:27: error: invalid use of non-static member function ‘void js::wasm::BaseRegAlloc::allocFPU(js::jit::FloatRegister)’
  937 |     return RegF32(allocFPU<MIRType::Float32>());
      |                   ~~~~~~~~^~~~~~~~~~~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:738:8: note: declared here
  738 |   void allocFPU(FloatRegister r) {
      |        ^~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:937:46: error: expected primary-expression before ‘)’ token
  937 |     return RegF32(allocFPU<MIRType::Float32>());
      |                                              ^
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp: In member function ‘js::wasm::RegF64 js::wasm::BaseRegAlloc::needF64()’:
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:951:27: error: invalid use of non-static member function ‘void js::wasm::BaseRegAlloc::allocFPU(js::jit::FloatRegister)’
  951 |     return RegF64(allocFPU<MIRType::Double>());
      |                   ~~~~~~~~^~~~~~~~~~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:738:8: note: declared here
  738 |   void allocFPU(FloatRegister r) {
      |        ^~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:951:18: error: expected primary-expression before ‘(’ token
  951 |     return RegF64(allocFPU<MIRType::Double>());
      |                  ^
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:951:27: error: invalid use of non-static member function ‘void js::wasm::BaseRegAlloc::allocFPU(js::jit::FloatRegister)’
  951 |     return RegF64(allocFPU<MIRType::Double>());
      |                   ~~~~~~~~^~~~~~~~~~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:738:8: note: declared here
  738 |   void allocFPU(FloatRegister r) {
      |        ^~~~~~~~
/home/sharkcz/projects/firefox/js/src/wasm/WasmBaselineCompile.cpp:951:45: error: expected primary-expression before ‘)’ token
  951 |     return RegF64(allocFPU<MIRType::Double>());
      |                                             ^
gmake[4]: *** [/home/sharkcz/projects/firefox/config/rules.mk:725: Unified_cpp_js_src_wasm0.o] Chyba 1
gmake[4]: Opouští se adresář „/home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu/js/src/wasm“
gmake[3]: *** [/home/sharkcz/projects/firefox/config/recurse.mk:72: js/src/wasm/target-objects] Chyba 2
gmake[3]: Opouští se adresář „/home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu“
gmake[2]: *** [/home/sharkcz/projects/firefox/config/recurse.mk:34: compile] Chyba 2
gmake[2]: Opouští se adresář „/home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu“
gmake[1]: *** [/home/sharkcz/projects/firefox/config/rules.mk:384: default] Chyba 2
gmake[1]: Opouští se adresář „/home/sharkcz/projects/firefox/obj-aarch64-unknown-linux-gnu“
gmake: *** [client.mk:125: build] Chyba 2
10 compiler warnings present.

Dan Horák

Reporter

Updated

•

4 years ago

Regressions: 1609381

Lars T Hansen [:lth]

Assignee

Comment 1

•

4 years ago

Patches welcome. gcc is not something we test with any longer.

Dan Horák

Reporter

Comment 2

•

4 years ago

I have asked our gcc/g++ experts in the linked Fedora bug.

URL: https://bugzilla.redhat.com/show_bug....

Dan Horák

Reporter

Comment 3

•

4 years ago

and https://github.com/mozilla/gecko-dev/commit/71597faac0fde4f608a60dd610d0cefac4972cc3.patch is the result

Lars T Hansen [:lth]

Assignee

Comment 4

•

4 years ago

Thanks! I'll try to get this landed soon.

Lars T Hansen [:lth]

Assignee

Comment 5

•

4 years ago

Attached file Bug 1677690 - Fix GCC 9 / GCC 10 build failure in WasmBaselineCompile.cpp. r?mgaudet — Details

GCC does not yet implement C++ Core DR 727, so explicit
specializations at class scope aren't allowed. This patch replaces it
with a different C++17 feature: using if-constexpr inside the function
body and checking the template argument there.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1677690
and https://bugzilla.redhat.com/show_bug.cgi?id=1897675

Patch authored by Jonathan Wakely <jwakely@redhat.com>

Phabricator Automation

Updated

•

4 years ago

Assignee: nobody → lhansen

tt_1

Comment 6

•

4 years ago

thanks for the patch, it fixes the bug for me on aarch64-linux and doesn't break armv7-linux at compile time.

Pulsebot

Comment 7

•

4 years ago

Pushed by lhansen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ebe83e59605b Fix GCC 9 / GCC 10 build failure in WasmBaselineCompile.cpp. r=mgaudet

Lars T Hansen [:lth]

Assignee

Updated

•

4 years ago

Severity: -- → S4

Status: UNCONFIRMED → ASSIGNED

Ever confirmed: true

Priority: -- → P3

Cosmin Sabou [:CosminS]

Comment 8

•

4 years ago

bugherder

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

Status: ASSIGNED → RESOLVED

Closed: 4 years ago

status-firefox85: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → 85 Branch

tt_1

Comment 9

•

4 years ago

can the fix please be uplifted to firefox-84 beta branch?

Lars T Hansen [:lth]

Assignee

Comment 10

•

4 years ago

Ryan, is there still time to uplift this to beta 84, see comment 9?

Flags: needinfo?(ryanvm)

Ryan VanderMeulen [:RyanVM]

Comment 11

•

4 years ago

Sure, go ahead and nominate.

status-firefox83: --- → wontfix

status-firefox84: --- → affected

status-firefox-esr78: --- → unaffected

Flags: needinfo?(ryanvm) → needinfo?(lhansen)

Keywords: regression

Regressed by: 1609381

No longer regressions: 1609381

BMO Automation

Updated

•

4 years ago

Has Regression Range: --- → yes

Lars T Hansen [:lth]

Assignee

Comment 12

•

4 years ago

Comment on attachment 9189189 [details]
Bug 1677690 - Fix GCC 9 / GCC 10 build failure in WasmBaselineCompile.cpp. r?mgaudet

Beta/Release Uplift Approval Request

User impact if declined: Pain point for redistributors who build with GCC; uplift requested by them.
Is this code covered by automated tests?: Yes
Has the fix been verified in Nightly?: Yes
Needs manual test from QE?: No
If yes, steps to reproduce:
List of other uplifts needed: None
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): It is a template hack that works around a GCC bug.
String changes made/needed:

Flags: needinfo?(lhansen)

Attachment #9189189 - Flags: approval-mozilla-beta?

Ryan VanderMeulen [:RyanVM]

Comment 13

•

4 years ago

Comment on attachment 9189189 [details]
Bug 1677690 - Fix GCC 9 / GCC 10 build failure in WasmBaselineCompile.cpp. r?mgaudet

Approved for 84.0b7.

Attachment #9189189 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Ryan VanderMeulen [:RyanVM]

Comment 14

•

4 years ago

bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/717a5833137e

status-firefox84: affected → fixed

tt_1

Comment 15

•

4 years ago

so by release of 84.0, --enable-rust-simd will be possible for linux-aarch64 with gcc?