Bump min browser versions such they all support llvm default features #17690

sbc100 · 2022-08-19T21:31:49Z

LLVM and binaryen updated their defaults to include sign extension and mutable globals: https://reviews.llvm.org/D125728.

According to https://webassembly.org/roadmap/ the min versions required for sign extension are:

firefox: 62
safai: 14.1
chrome: 71

And for mutable globals:

firefox: 61
safai: ??
chrome: 72

Since the minimum firefox and chrome versions are already 65 and 74, the only version we need to bump is the safari
version which needs to go from 12.0 to 14.1.

curiousdannii · 2022-08-20T01:55:47Z

If Edge is going to not be supported by default any more I think that definitely warrants an entry in the changelog, and it would be helpful if it could also say what settings to change in order to support Edge again.

juj · 2022-08-20T09:32:01Z

Bumped on the conversation thread at WebAssembly/website#187 (comment) to ask help to enable developers figure what these kinds of changes mean, without expecting everyone individually to become VM product history detectives.

The change on calling Edge not supported is probably an accident? I wonder if this is confusing old Edge ChakraCore with current Chromium based Edge? Edge should certainly still be supported.

Also, doesn't the upstream LLVM change mean that if one now compiles Emscripten code with, say, -sMIN_SAFARI_VERSION=120000, the output from Emscripten will be a wasm binary that will not actually run on Safari 12? (since Emscripten [after this PR] does not adjust its behavior depending on which MIN_version fields are in use)

The right fix would be to:

either make Emscripten error out if any of the -sMIN_version fields are passed on the command line that are too old for Emscripten to be able to produce a working wasm binary, or
if it is still possible to adjust upstream LLVM behavior to be backwards compatible (via -f? flags?), then either a) automatically make Emscripten fallback to such compatible flags depending on the -sMIN_version config desired (optionally with warnings), or b) issue warnings, or errors that the generated build won't work in the desired -sMIN_version targets.

src/settings.js

juj · 2022-08-20T09:36:09Z

src/settings.js

@@ -1743,7 +1743,7 @@ var MIN_FIREFOX_VERSION = 65;
 // see https://github.com/emscripten-core/emscripten/pull/7191.
 // MAX_INT (0x7FFFFFFF, or -1) specifies that target is not supported.
 // [link]
-var MIN_SAFARI_VERSION = 120000;
+var MIN_SAFARI_VERSION = 150000;


Isn't Safari 15 super recent, like July 2021? This would make Emscripten support only 1 year old Safari by default? That might cause a lot of surprises.

I mean, I am not opposed to this change as long as we can manually and explicitly fall back, but for this kind of change, we should be super loud and highlight this change every place possible, so it is not a surprise that Emscripten output will no longer work on every browser and phone that supports Wasm.

Yes, this one surprises me too. It's less than a year old if wikipedia is correct.

The larger question is market share, I guess. I don't actually know how best to estimate that, but caniuse/statcounter suggests 15 is by far the most popular, but 14 still has a small but significant amount - maybe 1/5 to 1/10th of 15's share. That's enough to worry me.

Has this been taken into account on the LLVM side? If not maybe we can discuss that there.

@dschuff are we sure that bulk-memory and non-trapping-float-to-int really depend on Safari 15?

This is sourced from Safari's release notes: https://developer.apple.com/documentation/safari-release-notes/safari-15-release-notes

juj · 2022-08-20T09:37:56Z

Also we should definitely have a ChangeLog entry documenting each of the minimum version bumps.

Btw, it would be great to add a MIN_NODEJS_VERSION field at some point in the future, given that there are a lot of version related things at play when targeting Node as well. Originally I was only concerned about browsers when creating the MIN_x_VERSION fields.

sbc100 · 2022-08-20T14:45:24Z

My apologies for now writing more details in PR entry, and in the ChangeLog. Obviously this is fairly big change and requires a loud announcement in the ChangeLog and possibly a lot more discussion here before we land it.

sbc100 · 2022-08-20T14:48:40Z

either make Emscripten error out if any of the -sMIN_version fields are passed on the command line that are too old for Emscripten to be able to produce a working wasm binary, or

if it is still possible to adjust upstream LLVM behavior to be backwards compatible (via -f? flags?), then either a) automatically make Emscripten fallback to such compatible flags depending on the -sMIN_version config desired (optionally with warnings), or b) issue warnings, or errors that the generated build won't work in the desired -sMIN_version targets.

Sorry should have given more context. The plan is to land #17689, or something like it, first so that emscripten will automatically set the required feature flags based on the -sMIN settings. Only then would we want bumping the MIN flags, or update our LLVM version to one that sets them by default.

juj · 2022-08-25T08:43:56Z

Gotcha, thanks! That plan sounds perfect.

sbc100 · 2022-11-15T19:08:58Z

I assume you realize this is the old EdgeHTML browser that predates the chromium-based edge that has been around for many years now?

sbc100 · 2022-11-15T19:21:03Z

PTAL.

In the end only two features were added so this change is a lot more minimal now.

sbc100 · 2022-11-15T19:21:41Z

We need to land this, along with #17689 before we can pull in new llvm versions.

kripken · 2022-11-15T19:25:43Z

Comments above suggest #17689 should land first, but hasn't?

kripken · 2022-11-15T19:27:36Z

ChangeLog.md

+  was updated from 12.0 to 14.1, and support for the old EdgeHTML engine
+  (`MIN_EDGE_VERSION`) was removed by default.  If you want to continue to
+  support older engines you can use these settings to specify alternative minium
+  versions which will result in the new proposals being disabled. (#17690)


Maybe specify the actual flags, -sMIN_SAFARI_VERSION=120000, -sMIN_EDGE_VERSION=44, for those that want the old behavior, so they don't need to figure those out?

sbc100 · 2022-11-15T20:12:32Z

Comments above suggest #17689 should land first, but hasn't?

Sure we can land #17689 first. I've updated it now. They are little interdependent, but that order is not super important I think.

juj · 2022-11-16T11:28:01Z

src/settings.js

@@ -1771,7 +1771,7 @@ var MIN_IE_VERSION = 0x7FFFFFFF;
 // Edge 44.17763 was released on November 13, 2018
 // MAX_INT (0x7FFFFFFF, or -1) specifies that target is not supported.
 // [link]
-var MIN_EDGE_VERSION = 44;
+var MIN_EDGE_VERSION = 0x7FFFFFFF;


I think this is still wrong, Edge definitely should be supported by default.

Do you understand this this setting does not refer to the current Edge browser (which is based on chromium) but the many years old EdgeHTML engine which does exist anymore?

No I don't. When I wrote this code, I fully intended it to refer to Edge as a product, there is no reason to limit its meaning to old EdgeHTML. The version numbering they devised for old vs new Edge conveniently follows a scheme that can be utilized here. There is no reason not to.

I see, I will re-phrase. I assume you are OK with dropping support for the old EdgeHTML browser as part of this chagne?

If we want to continue to refer to the modern Edge browser then I suppose we would want to add an Edge column to the https://webassembly.org/roadmap/ page?

Do the first chromium-based Edge release was based on chrome 79 it seems: https://en.wikipedia.org/wiki/Microsoft_Edge#New_Edge_release_history. And from then on the edge version numbers pretty much match up with chromium version numbers. Given that, does it still make sense to have two different version numbers?

Yes, certainly, no trouble dropping support for old EdgeHTML itself.

There is a valid question though how 1:1 the new Edge features are with Chromium features. I don't actually know.

We could argue for the case that if new Edge is 1:1 with their version numbers and features with Chromium version numbers and features, it would not make sense to duplicate these version fields over to two options, and, say, MIN_CHROME_VERSION=100 and MIN_EDGE_VERSION=100.

One approach to move forward might be to add a comment along the lines of

// For now, this field refers to supporting old EdgeHTML browser, but if discrepancies between Chromium-based Edge and Chrome surface, then this field can be updated to refer to new Edge options as well.

so we'd keep this field frozen and reinstate when/if it becomes useful.

Sounds like a plan

OK, I added to comment to make it clear the this currently refers to EdgeHTML but this might change if modern-Edge and Chromium start to diverge.

src/settings.js

LLVM and binaryen updated their defaults to include sign extension and mutable globals: https://reviews.llvm.org/D125728. According to https://webassembly.org/roadmap/ the min versions required for sign extension are: - firefox: 62 - safai: 14.1 - chrome: 71 And for mutable globals: - firefox: 61 - safai: ?? - chrome: 72 I believe safari implement mutable globals since the beginning which is perhaps why there is no min version specified. Since the minimum firefox and chrome versions are already 65 and 74, the only version we need to bump is the safari version which needs to go from 12.0 to 14.1. Also, drop support for the old EdgeHTML engine by default, we we don't know the status of these features on that engine (we also don't have any testing at all of this platform).

dasa · 2022-11-16T17:01:17Z

src/settings.js

@@ -1768,10 +1770,17 @@ var MIN_IE_VERSION = 0x7FFFFFFF;
 // Specifies the oldest version of Edge (EdgeHTML, the non-Chromium based
 // flavor) to target. E.g. pass -sMIN_EDGE_VERSION=40 to drop support for
 // EdgeHTML 39 and older.
-// Edge 44.17763 was released on November 13, 2018
+// EdgeHTML 44.17763 was released on November 13, 2018
+// EdgeHTML was completely in April 2021 and replaced by the current


completely? completed?

That should be removed.. I edited from completely removed and deleted the wrong word .. oops.

sbc100 · 2022-11-16T17:12:19Z

Oops, I didn't mean to land that without a final lgtm from @juj. @juj are you happy with the version that landed? I can do a followup or revert if not.

juj · 2022-11-17T12:48:12Z

src/settings.js

+// EdgeHTML was completely in April 2021 and replaced by the current
+// Chromium-based Edge.
+// Since version 79, Edge version numbers have mirrored chromium version
+// numbers, so it no longer makes sense specify MIN_EDGE_VERSION independenly.


independenly

juj · 2022-11-17T12:48:21Z

src/settings.js

+// Chromium-based Edge.
+// Since version 79, Edge version numbers have mirrored chromium version
+// numbers, so it no longer makes sense specify MIN_EDGE_VERSION independenly.
+// If Chromium and Edge ever start to diverage this setting may be revived with


juj · 2022-11-17T12:48:43Z

LGTM!

sbc100 requested review from kripken and dschuff August 19, 2022 21:31

sbc100 mentioned this pull request Aug 19, 2022

Add sign-ext lowering pass when target browser doesn't support it #17689

Merged

juj mentioned this pull request Aug 20, 2022

Please change roadmap checkmark chart to caniuse style "first introduced in" WebAssembly/website#187

Closed

juj reviewed Aug 20, 2022

View reviewed changes

src/settings.js Show resolved Hide resolved

juj reviewed Aug 20, 2022

View reviewed changes

sbc100 mentioned this pull request Nov 14, 2022

Add a sign extension lowering pass WebAssembly/binaryen#5250

Closed

sbc100 changed the title ~~Bump min browser versions such they all support llvm default features~~ Bump minimum safari version to match new llvm default Nov 15, 2022

sbc100 force-pushed the bump_versions branch from e9c0804 to 1957afc Compare November 15, 2022 19:08

sbc100 changed the title ~~Bump minimum safari version to match new llvm default~~ Bump min browser versions such they all support llvm default features Nov 15, 2022

sbc100 requested a review from juj November 15, 2022 19:08

sbc100 force-pushed the bump_versions branch 3 times, most recently from 3fad616 to a183408 Compare November 15, 2022 19:17

sbc100 force-pushed the bump_versions branch from a183408 to fa6faee Compare November 15, 2022 19:22

kripken reviewed Nov 15, 2022

View reviewed changes

sbc100 force-pushed the bump_versions branch from fa6faee to 492db5e Compare November 15, 2022 20:14

kripken approved these changes Nov 15, 2022

View reviewed changes

sbc100 enabled auto-merge (squash) November 15, 2022 22:28

juj reviewed Nov 16, 2022

View reviewed changes

src/settings.js Show resolved Hide resolved

sbc100 force-pushed the bump_versions branch from 492db5e to 29c1177 Compare November 16, 2022 14:59

sbc100 force-pushed the bump_versions branch from 29c1177 to ed449df Compare November 16, 2022 15:22

sbc100 merged commit 91e35f3 into main Nov 16, 2022

sbc100 deleted the bump_versions branch November 16, 2022 16:54

dasa reviewed Nov 16, 2022

View reviewed changes

juj reviewed Nov 17, 2022

View reviewed changes

ricardperez mentioned this pull request Jun 26, 2023

Safari iOS 14.3 fails to load wasm even with -sMIN_SAFARI_VERSION=120000 #19718

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump min browser versions such they all support llvm default features #17690

Bump min browser versions such they all support llvm default features #17690

sbc100 commented Aug 19, 2022 •

edited

Loading

curiousdannii commented Aug 20, 2022

juj commented Aug 20, 2022 •

edited

Loading

juj Aug 20, 2022

juj Aug 20, 2022

kripken Aug 22, 2022 •

edited

Loading

sbc100 Aug 22, 2022

dschuff Aug 23, 2022

juj commented Aug 20, 2022

sbc100 commented Aug 20, 2022

sbc100 commented Aug 20, 2022

juj commented Aug 25, 2022

sbc100 commented Nov 15, 2022

sbc100 commented Nov 15, 2022

sbc100 commented Nov 15, 2022

kripken commented Nov 15, 2022

kripken Nov 15, 2022

sbc100 commented Nov 15, 2022

juj Nov 16, 2022

sbc100 Nov 16, 2022

juj Nov 16, 2022

sbc100 Nov 16, 2022

sbc100 Nov 16, 2022

sbc100 Nov 16, 2022

juj Nov 16, 2022

sbc100 Nov 16, 2022

sbc100 Nov 16, 2022

dasa Nov 16, 2022

sbc100 Nov 16, 2022

sbc100 commented Nov 16, 2022

juj Nov 17, 2022

juj Nov 17, 2022

juj commented Nov 17, 2022

Bump min browser versions such they all support llvm default features #17690

Bump min browser versions such they all support llvm default features #17690

Conversation

sbc100 commented Aug 19, 2022 • edited Loading

curiousdannii commented Aug 20, 2022

juj commented Aug 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kripken Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juj commented Aug 20, 2022

sbc100 commented Aug 20, 2022

sbc100 commented Aug 20, 2022

juj commented Aug 25, 2022

sbc100 commented Nov 15, 2022

sbc100 commented Nov 15, 2022

sbc100 commented Nov 15, 2022

kripken commented Nov 15, 2022

Choose a reason for hiding this comment

sbc100 commented Nov 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbc100 commented Nov 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juj commented Nov 17, 2022

sbc100 commented Aug 19, 2022 •

edited

Loading

juj commented Aug 20, 2022 •

edited

Loading

kripken Aug 22, 2022 •

edited

Loading