Closed Bug 1677690 Opened 2 months ago Closed 2 months ago

build fails after introducing SIMD support on aarch64

Categories

(Core :: Javascript: WebAssembly, defect, P3)

ARM64
Linux
defect

Tracking

()

RESOLVED FIXED
85 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox83 --- wontfix
firefox84 --- fixed
firefox85 --- fixed

People

(Reporter: dan, Assigned: lth)

References

(Regression, )

Details

(Keywords: regression)

Attachments

(1 file)

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.
Regressions: 1609381

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

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

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

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>

Assignee: nobody → lhansen

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

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
Severity: -- → S4
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P3
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch

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

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

Flags: needinfo?(ryanvm)

Sure, go ahead and nominate.

Flags: needinfo?(ryanvm) → needinfo?(lhansen)
Keywords: regression
Regressed by: 1609381
No longer regressions: 1609381

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?

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+

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

You need to log in before you can comment on or make changes to this bug.