Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

REGRESSION(257865@main): B3Validate.cpp: fix !ENABLE(WEBASSEMBLY_B3JIT) #8696

Closed
wants to merge 1 commit into from
Closed

REGRESSION(257865@main): B3Validate.cpp: fix !ENABLE(WEBASSEMBLY_B3JIT) #8696

wants to merge 1 commit into from

Conversation

ThomasDevoogdt
Copy link
Contributor

@ThomasDevoogdt ThomasDevoogdt commented Jan 16, 2023 •
edited by Ahmad-S792 

REGRESSION(257865@main): B3Validate.cpp: fix !ENABLE(WEBASSEMBLY_B3JIT)
https://bugs.webkit.org/show_bug.cgi?id=250681

Reviewed by NOBODY (OOPS!).

WasmTypeDefinition.h isn't included if not ENABLE(WEBASSEMBLY_B3JIT). Also, toB3Type and simdScalarType are not defined if it is included.

Signed-off-by: Thomas Devoogdt

30e1d5e

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 mac-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🧪 mac-wk2-stress ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@ThomasDevoogdt ThomasDevoogdt requested a review from a team as a code owner January 16, 2023 16:05
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash df00fec)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ✅ 🛠 wpe   🛠 🧪 win
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ⏳ 🧪 gtk-wk2
✅ 🧪 webkitpy ❌ 🧪 api-ios ❌ 🧪 mac-wk1 ⏳ 🧪 api-gtk
  🛠 🧪 jsc ❌ 🛠 tv ❌ 🧪 mac-wk2 ❌ 🛠 jsc-armv7
❌ 🛠 🧪 jsc-arm64 ❌ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2 ❌ 🧪 jsc-armv7-tests
✅ 🧪 services ❌ 🛠 watch ❌ 🧪 mac-wk2-stress ✅ 🛠 jsc-mips
❌ 🛠 watch-sim   🧪 jsc-mips-tests

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 16, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash f3343d3)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 mac-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🧪 mac-wk2-stress ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@ThomasDevoogdt ThomasDevoogdt mentioned this pull request Jan 18, 2023
Merged
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 94b4918)

Misc iOS, tvOS & watchOS macOS Linux Windows
⏳ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 🧪 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🛠 gtk ⏳ 🛠 wincairo
⏳ 🧪 webkitperl ⏳ 🧪 ios-wk2   🧪 api-mac ⏳ 🧪 gtk-wk2
  🧪 api-ios   🧪 mac-wk1 ⏳ 🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv   🧪 mac-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64 ⏳ 🛠 tv-sim ⏳ 🧪 mac-AS-debug-wk2   🧪 jsc-armv7-tests
✅ 🛠 watch   🧪 mac-wk2-stress ✅ 🛠 jsc-mips
⏳ 🛠 watch-sim   🧪 jsc-mips-tests

@ThomasDevoogdt
REGRESSION(257865@main): B3Validate.cpp: fix !ENABLE(WEBASSEMBLY_B3JIT)
30e1d5e 
https://bugs.webkit.org/show_bug.cgi?id=250681

Reviewed by NOBODY (OOPS!).

WasmTypeDefinition.h isn't included if not ENABLE(WEBASSEMBLY_B3JIT).
Also, toB3Type and simdScalarType are not defined if it is included.

Signed-off-by: Thomas Devoogdt <thomas.devoogdt@barco.com>
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 30e1d5e)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 mac-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🧪 mac-wk2-stress ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@Constellation
Copy link
Member

If B3 is compiled, WEBASSEMBLY_B3JIT needs to be enabled.

Constellation
Constellation requested changes Feb 8, 2023
View reviewed changes
Copy link
Member

@Constellation Constellation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r- based on comment.

@ThomasDevoogdt
Copy link
Contributor Author

If B3 is compiled, WEBASSEMBLY_B3JIT needs to be enabled.

I found out about the ENABLE_JIT flag which sets some dependencies right. Controlling the individual ENABLE_WEBASSEMBLY{_B3JIT} flags almost always causes a bunch of compile errors. And I do not per se need JIT, so just disable it all. To you to decide if this issue can be closed, or if some commit change is required, feel free to push to this PR.

I won't put more time into getting the -DENABLE_WEBASSEMBLY=OFF flag working.

@ThomasDevoogdt ThomasDevoogdt mentioned this pull request Feb 24, 2023
Closed
@Ahmad-S792 Ahmad-S792 added the WebAssembly For bugs in JavaScript WebAssembly label Mar 14, 2023
@ThomasDevoogdt ThomasDevoogdt deleted the bugfix/257865@main branch September 26, 2023 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Constellation Constellation Constellation requested changes

Assignees

@ThomasDevoogdt ThomasDevoogdt

Labels
merging-blocked Applied to prevent a change from being merged WebAssembly For bugs in JavaScript WebAssembly
Projects
None yet
Milestone
No milestone
5 participants
@ThomasDevoogdt @webkit-early-warning-system @Constellation @Ahmad-S792 @webkit-ews-buildbot