Bug 250681

Summary: REGRESSION(257865@main): [RISCV64] B3Validate.cpp: fix !ENABLE(WEBASSEMBLY_B3JIT)
Product: WebKit Reporter: Thomas Devoogdt <thomas.devoogdt>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: bugs-noreply, clopez, jbicha, jsims, justin_michaud, mcatanzaro, webkit-bug-importer, ysuzuki, zan, zdobersek
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=238007

Description Thomas Devoogdt 2023-01-16 08:01:12 PST
WasmTypeDefinition.h isn't included if not ENABLE(WEBASSEMBLY_B3JIT).
Also, toB3Type and simdScalarType are not defined if it is included.


[ 20%] Building CXX object Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/__/__/JavaScriptCore/DerivedSources/unified-sources/UnifiedSource-23a5fd0e-13.cpp.o
In file included from /home/thomas/buildroot/output/build/webkitgtk-2.39.4/JavaScriptCore/DerivedSources/unified-sources/UnifiedSource-23a5fd0e-9.cpp:5:
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp: In member function ‘void JSC::B3::{anonymous}::Validater::run()’:
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:455:58: error: ‘simdScalarType’ is not a member of ‘JSC::Wasm’
  455 |                 VALIDATE(value->type() == toB3Type(Wasm::simdScalarType(value->asSIMDValue()->simdLane())), ("At ", *value));
      |                                                          ^~~~~~~~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:63:13: note: in definition of macro ‘VALIDATE’
   63 |         if (condition)                                                  \
      |             ^~~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:455:43: error: ‘toB3Type’ was not declared in this scope
  455 |                 VALIDATE(value->type() == toB3Type(Wasm::simdScalarType(value->asSIMDValue()->simdLane())), ("At ", *value));
      |                                           ^~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:63:13: note: in definition of macro ‘VALIDATE’
   63 |         if (condition)                                                  \
      |             ^~~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:463:68: error: ‘simdScalarType’ is not a member of ‘JSC::Wasm’
  463 |                 VALIDATE(value->child(1)->type() == toB3Type(Wasm::simdScalarType(value->asSIMDValue()->simdLane())), ("At ", *value));
      |                                                                    ^~~~~~~~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:63:13: note: in definition of macro ‘VALIDATE’
   63 |         if (condition)                                                  \
      |             ^~~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:463:53: error: ‘toB3Type’ was not declared in this scope
  463 |                 VALIDATE(value->child(1)->type() == toB3Type(Wasm::simdScalarType(value->asSIMDValue()->simdLane())), ("At ", *value));
      |                                                     ^~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:63:13: note: in definition of macro ‘VALIDATE’
   63 |         if (condition)                                                  \
      |             ^~~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:478:68: error: ‘simdScalarType’ is not a member of ‘JSC::Wasm’
  478 |                 VALIDATE(value->child(0)->type() == toB3Type(Wasm::simdScalarType(value->asSIMDValue()->simdLane())), ("At ", *value));
      |                                                                    ^~~~~~~~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:63:13: note: in definition of macro ‘VALIDATE’
   63 |         if (condition)                                                  \
      |             ^~~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:478:53: error: ‘toB3Type’ was not declared in this scope
  478 |                 VALIDATE(value->child(0)->type() == toB3Type(Wasm::simdScalarType(value->asSIMDValue()->simdLane())), ("At ", *value));
      |                                                     ^~~~~~~~
/home/thomas/buildroot/output/build/webkitgtk-2.39.4/Source/JavaScriptCore/b3/B3Validate.cpp:63:13: note: in definition of macro ‘VALIDATE’
   63 |         if (condition)                                                  \
      |             ^~~~~~~~~
Comment 1 Michael Catanzaro 2023-03-01 13:36:27 PST
*** Bug 252758 has been marked as a duplicate of this bug. ***
Comment 2 Justin Michaud 2023-03-01 20:53:27 PST
I think that enabling WebAssembly but not having ENABLE_WEBASSEMBLY_B3JIT is a very unusual configuration, unsupported by any other platform. I think the solution here is just to disable WebAssembly, this isn't a configuration we would like to support.

In particular, we are landing a new wasm tier that will also be a pain to define out for this very specific case. 

If you want to make it work, go ahead, but my guess is that WASM on risc-v is not a super high priority for y'all at the moment.
Comment 3 Thomas Devoogdt 2023-03-01 21:39:53 PST
Hi, 

Yes, I know, In the meantime I've adapted the build commands. The original PR can be found here: https://github.com/WebKit/WebKit/pull/8696. (If any interest, otherwise just close it.)

Kr

Thomas Devoogdt
Comment 4 Michael Catanzaro 2023-03-02 07:28:27 PST
I did not know about https://github.com/WebKit/WebKit/pull/8696 because this bug did not contain any link to it!

(In reply to Justin Michaud from comment #2)
> I think that enabling WebAssembly but not having ENABLE_WEBASSEMBLY_B3JIT is
> a very unusual configuration, unsupported by any other platform.

I see Yusuke says, from https://github.com/WebKit/WebKit/pull/8696: "If B3 is compiled, WEBASSEMBLY_B3JIT needs to be enabled." And B3 is a dependency of WebAssembly. I see the non-RISCV64 guards in PlatformEnable.h also attempt to enforce this dependency. So why are ENABLE_WEBASSEMBLY and ENABLE_WEBASSEMBLY_B3JIT separate flags, then? If this is all true, then we should surely get rid of ENABLE_WEBASSEMBLY_B3JIT, right?

Anyway, sounds like we'll need to revert the PlatformEnable.h changes from 255090@main "[JSC] RISCV64 support for WebAssembly". Zan, what do you think?
Comment 5 Jeremy Bicha 2023-03-09 13:38:03 PST
Hi, the riscv64 build hasn't been fixed for webkitgtk 2.39.91. This is a problem for Ubuntu.

I tried reverting https://commits.webkit.org/255090@main but it still fails to build.

[1519/6825] cd /<<PKGBUILDDIR>>/build-soup2/JavaScriptCore/DerivedSources && /usr/bin/cmake -E env CMAKE_CXX_COMPILER_ID=GNU GCC_OFFLINEASM_SOURCE_MAP=OFF /usr/bin/ruby /<<PKGBUILDDIR>>/Source/JavaScriptCore/offlineasm/asm.rb -I/<<PKGBUILDDIR>>/build-soup2/JavaScriptCore/DerivedSources/ /<<PKGBUILDDIR>>/Source/JavaScriptCore/llint/LowLevelInterpreter.asm /<<PKGBUILDDIR>>/build-soup2/bin/LLIntOffsetsExtractor /<<PKGBUILDDIR>>/build-soup2/JavaScriptCore/DerivedSources/LLIntAssembly.h normal --binary-format=ELF && /usr/bin/cmake -E touch_nocreate /<<PKGBUILDDIR>>/build-soup2/JavaScriptCore/DerivedSources/LLIntAssembly.h
FAILED: JavaScriptCore/DerivedSources/LLIntAssembly.h /<<PKGBUILDDIR>>/build-soup2/JavaScriptCore/DerivedSources/LLIntAssembly.h 
cd /<<PKGBUILDDIR>>/build-soup2/JavaScriptCore/DerivedSources && /usr/bin/cmake -E env CMAKE_CXX_COMPILER_ID=GNU GCC_OFFLINEASM_SOURCE_MAP=OFF /usr/bin/ruby /<<PKGBUILDDIR>>/Source/JavaScriptCore/offlineasm/asm.rb -I/<<PKGBUILDDIR>>/build-soup2/JavaScriptCore/DerivedSources/ /<<PKGBUILDDIR>>/Source/JavaScriptCore/llint/LowLevelInterpreter.asm /<<PKGBUILDDIR>>/build-soup2/bin/LLIntOffsetsExtractor /<<PKGBUILDDIR>>/build-soup2/JavaScriptCore/DerivedSources/LLIntAssembly.h normal --binary-format=ELF && /usr/bin/cmake -E touch_nocreate /<<PKGBUILDDIR>>/build-soup2/JavaScriptCore/DerivedSources/LLIntAssembly.h
/<<PKGBUILDDIR>>/Source/JavaScriptCore/offlineasm/ast.rb:958:in `lowerDefault': Unhandled opcode storev at WebAssembly.asm:512 (due to WebAssembly.asm:512) (LoweringError)


I can do test builds with my Launchpad PPA that has riscv64 enabled (usually PPAs don't have that available).
Comment 6 Zan Dobersek 2023-03-10 05:19:17 PST
The generalized set of build fixes is being landed under bug #253700:
https://github.com/WebKit/WebKit/pull/11358

WebAssembly was enabled because its support in LLInt was being developed. But (as far as I remember) enabling it also required enabling the B3 JIT feature, and WEBASSEMBLY_B3JIT was left out because compiling without it worked at that point.

We intentionally enable FTL-level JIT (B3) for RISCV64 because we want to avoid these weird build-time situations where RISCV64 differs from X86_64 and ARM64, and as a result such build errors creep up. FTL/B3 is still disabled at build-time.
Comment 7 Michael Catanzaro 2023-03-10 07:01:15 PST
Thanks Zan!

*** This bug has been marked as a duplicate of bug 253700 ***