Enable nontrapping-fp by default (#23007)

dschuff · web-flow · commit d23a20d35938 · 2024-12-19T10:57:21.000-08:00
Enable nontrapping-fptoint by lowering the browser target requirement.
Once all 3 Safari 15 features have been enabled, I will reset
the requirement correctly.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -20,6 +20,14 @@ See docs/process.md for more on how version tagging works.
 
 3.1.75 (in development)
 -----------------------
+- The Wasm nontrapping-fptoint feature has been enabled by default. clang will
+  generate nontrapping (saturating) float-to-int conversion instructions for
+  C typecasts. This should have no effect on programs that do not have
+  undefined behavior but if the casted floating-point value is outside the range
+  of the target integer type, the result will be a number of the max or min value
+  instead of a trap. This also results in a small code size improvement because
+  of details of the LLVM IR semantics. This feature can be disabled in clang with
+  the -mno-nontrapping-fptoint flag. (#23007)
 - The `WASM_BIGINT` feature has been enabled by default. This has the effect that
   Wasm i64 values are passed and returned between Wasm and JS as BigInt values
   rather than being split by Binaryen into pairs of Numbers. (#22993)
diff --git a/emcc.py b/emcc.py
@@ -392,8 +392,6 @@ def get_clang_flags(user_args):
     if not settings.BULK_MEMORY:
       flags.append('-mno-bulk-memory')
       flags.append('-mno-bulk-memory-opt')
-  if '-mnontrapping-fptoint' not in user_args and '-mno-nontrapping-fptoint' not in user_args:
-    flags.append('-mno-nontrapping-fptoint')
 
   if settings.RELOCATABLE and '-fPIC' not in user_args:
     flags.append('-fPIC')
@@ -1435,6 +1433,12 @@ def consume_arg_file():
                                     override=True)
     elif arg == '-mno-sign-ext':
       feature_matrix.disable_feature(feature_matrix.Feature.SIGN_EXT)
+    elif arg == '-mnontrappting-fptoint':
+      feature_matrix.enable_feature(feature_matrix.Feature.NON_TRAPPING_FPTOINT,
+                                    '-mnontrapping-fptoint',
+                                    override=True)
+    elif arg == '-mno-nontrapping-fptoint':
+      feature_matrix.disable_feature(feature_matrix.Feature.NON_TRAPPING_FPTOINT)
     elif arg == '-fexceptions':
       # TODO Currently -fexceptions only means Emscripten EH. Switch to wasm
       # exception handling by default when -fexceptions is given when wasm
diff --git a/test/code_size/hello_webgl2_wasm.json b/test/code_size/hello_webgl2_wasm.json
@@ -3,8 +3,8 @@
   "a.html.gz": 328,
   "a.js": 4538,
   "a.js.gz": 2320,
-  "a.wasm": 10402,
-  "a.wasm.gz": 6703,
-  "total": 15394,
-  "total_gz": 9351
+  "a.wasm": 10206,
+  "a.wasm.gz": 6663,
+  "total": 15198,
+  "total_gz": 9311
 }
diff --git a/test/code_size/hello_webgl2_wasm2js.json b/test/code_size/hello_webgl2_wasm2js.json
@@ -1,8 +1,8 @@
 {
   "a.html": 346,
   "a.html.gz": 262,
-  "a.js": 22206,
-  "a.js.gz": 11589,
-  "total": 22552,
-  "total_gz": 11851
+  "a.js": 22202,
+  "a.js.gz": 11604,
+  "total": 22548,
+  "total_gz": 11866
 }
diff --git a/test/code_size/hello_webgl_wasm.json b/test/code_size/hello_webgl_wasm.json
@@ -3,8 +3,8 @@
   "a.html.gz": 328,
   "a.js": 4076,
   "a.js.gz": 2163,
-  "a.wasm": 10402,
-  "a.wasm.gz": 6703,
-  "total": 14932,
-  "total_gz": 9194
+  "a.wasm": 10206,
+  "a.wasm.gz": 6663,
+  "total": 14736,
+  "total_gz": 9154
 }
diff --git a/test/code_size/hello_webgl_wasm2js.json b/test/code_size/hello_webgl_wasm2js.json
@@ -1,8 +1,8 @@
 {
   "a.html": 346,
   "a.html.gz": 262,
-  "a.js": 21732,
-  "a.js.gz": 11419,
-  "total": 22078,
-  "total_gz": 11681
+  "a.js": 21728,
+  "a.js.gz": 11435,
+  "total": 22074,
+  "total_gz": 11697
 }
diff --git a/test/code_size/math_wasm.json b/test/code_size/math_wasm.json
@@ -3,8 +3,8 @@
   "a.html.gz": 380,
   "a.js": 110,
   "a.js.gz": 125,
-  "a.wasm": 2719,
-  "a.wasm.gz": 1675,
-  "total": 3381,
-  "total_gz": 2180
+  "a.wasm": 2695,
+  "a.wasm.gz": 1664,
+  "total": 3357,
+  "total_gz": 2169
 }
diff --git a/test/code_size/random_printf_wasm.json b/test/code_size/random_printf_wasm.json
@@ -1,6 +1,6 @@
 {
-  "a.html": 12597,
-  "a.html.gz": 6882,
-  "total": 12597,
-  "total_gz": 6882
+  "a.html": 12517,
+  "a.html.gz": 6871,
+  "total": 12517,
+  "total_gz": 6871
 }
diff --git a/test/code_size/random_printf_wasm2js.json b/test/code_size/random_printf_wasm2js.json
@@ -1,6 +1,6 @@
 {
   "a.html": 17195,
-  "a.html.gz": 7478,
+  "a.html.gz": 7480,
   "total": 17195,
-  "total_gz": 7478
+  "total_gz": 7480
 }
diff --git a/test/other/codesize/test_codesize_cxx_ctors1.size b/test/other/codesize/test_codesize_cxx_ctors1.size
@@ -1 +1 @@
-128974
+128914
diff --git a/test/other/codesize/test_codesize_cxx_ctors2.size b/test/other/codesize/test_codesize_cxx_ctors2.size
@@ -1 +1 @@
-128423
+128363
diff --git a/test/other/codesize/test_codesize_cxx_except.size b/test/other/codesize/test_codesize_cxx_except.size
@@ -1 +1 @@
-171008
+170948
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm.size b/test/other/codesize/test_codesize_cxx_except_wasm.size
@@ -1 +1 @@
-142223
+142163
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm_exnref.size b/test/other/codesize/test_codesize_cxx_except_wasm_exnref.size
@@ -1 +1 @@
-144810
+144750
diff --git a/test/other/codesize/test_codesize_cxx_lto.size b/test/other/codesize/test_codesize_cxx_lto.size
@@ -1 +1 @@
-121924
+121864
diff --git a/test/other/codesize/test_codesize_cxx_mangle.size b/test/other/codesize/test_codesize_cxx_mangle.size
@@ -1 +1 @@
-232517
+232457
diff --git a/test/other/codesize/test_codesize_cxx_noexcept.size b/test/other/codesize/test_codesize_cxx_noexcept.size
@@ -1 +1 @@
-131781
+131721
diff --git a/test/other/codesize/test_codesize_cxx_wasmfs.size b/test/other/codesize/test_codesize_cxx_wasmfs.size
@@ -1 +1 @@
-168928
+168868
diff --git a/test/other/codesize/test_codesize_hello_O0.size b/test/other/codesize/test_codesize_hello_O0.size
@@ -1 +1 @@
-15142
+15087
diff --git a/test/other/codesize/test_codesize_hello_O1.size b/test/other/codesize/test_codesize_hello_O1.size
@@ -1 +1 @@
-2634
+2655
diff --git a/test/other/codesize/test_codesize_minimal_O0.size b/test/other/codesize/test_codesize_minimal_O0.size
@@ -1 +1 @@
-1104
+1125
diff --git a/test/other/codesize/test_codesize_minimal_O1.size b/test/other/codesize/test_codesize_minimal_O1.size
@@ -1 +1 @@
-397
+418
diff --git a/test/other/codesize/test_codesize_minimal_pthreads.size b/test/other/codesize/test_codesize_minimal_pthreads.size
@@ -1 +1 @@
-19487
+19448
diff --git a/test/other/test_unoptimized_code_size.wasm.size b/test/other/test_unoptimized_code_size.wasm.size
@@ -1 +1 @@
-15142
+15087
diff --git a/test/other/test_unoptimized_code_size_no_asserts.wasm.size b/test/other/test_unoptimized_code_size_no_asserts.wasm.size
@@ -1 +1 @@
-12223
+12168
diff --git a/test/other/test_unoptimized_code_size_strict.wasm.size b/test/other/test_unoptimized_code_size_strict.wasm.size
@@ -1 +1 @@
-15142
+15087
diff --git a/test/test_other.py b/test/test_other.py
@@ -10461,7 +10461,7 @@ def compile(flags):
 
     compile(['-c'])
     verify_features_sec('bulk-memory', False)
-    verify_features_sec('nontrapping-fptoint', False)
+    verify_features_sec('nontrapping-fptoint', True)
     verify_features_sec('sign-ext', True)
     verify_features_sec('mutable-globals', True)
     verify_features_sec('multivalue', True)
@@ -10482,8 +10482,8 @@ def compile(flags):
     compile(['-sMIN_FIREFOX_VERSION=61', '-msign-ext'])
     verify_features_sec_linked('sign-ext', True)
 
-    compile(['-mnontrapping-fptoint', '-c'])
-    verify_features_sec('nontrapping-fptoint', True)
+    compile(['-mno-nontrapping-fptoint'])
+    verify_features_sec_linked('nontrapping-fptoint', False)
 
     # Setting this SAFARI_VERSION should enable bulk memory because it links in emscripten_memcpy_bulkmem
     # However it does not enable nontrapping-fptoint yet because it has no effect at compile time and
@@ -10493,7 +10493,7 @@ def compile(flags):
     verify_features_sec_linked('mutable-globals', True)
     verify_features_sec_linked('multivalue', True)
     verify_features_sec_linked('bulk-memory', True)
-    verify_features_sec_linked('nontrapping-fptoint', False)
+    verify_features_sec_linked('nontrapping-fptoint', True)
 
     compile(['-sMIN_SAFARI_VERSION=150000', '-mno-bulk-memory'])
     # -mno-bulk-memory at link time overrides MIN_SAFARI_VERSION
diff --git a/tools/feature_matrix.py b/tools/feature_matrix.py
@@ -46,7 +46,8 @@ class Feature(IntEnum):
   Feature.NON_TRAPPING_FPTOINT: {
     'chrome': 75,
     'firefox': 65,
-    'safari': 150000,
+    'safari': 140100, # TODO: Reset back to 150000 when the default changes
+    'node': 130000,
   },
   Feature.SIGN_EXT: {
     'chrome': 74,
diff --git a/tools/link.py b/tools/link.py
@@ -340,6 +340,9 @@ def get_binaryen_passes():
   if not feature_matrix.caniuse(feature_matrix.Feature.SIGN_EXT):
     logger.debug('lowering sign-ext feature due to incompatible target browser engines')
     passes += ['--signext-lowering']
+  # nontrapping-fp is enabled by default in llvm. Lower it away if requested.
+  if not feature_matrix.caniuse(feature_matrix.Feature.NON_TRAPPING_FPTOINT):
+    passes += ['--llvm-nontrapping-fptoint-lowering']
   if optimizing:
     passes += ['--post-emscripten']
     if settings.SIDE_MODULE:

Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,8 @@`
`1`	`1`	`{`
`2`	`2`	`"a.html": 346,`
`3`	`3`	`"a.html.gz": 262,`
`4`		`- "a.js": 22206,`
`5`		`- "a.js.gz": 11589,`
`6`		`- "total": 22552,`
`7`		`- "total_gz": 11851`
	`4`	`+ "a.js": 22202,`
	`5`	`+ "a.js.gz": 11604,`
	`6`	`+ "total": 22548,`
	`7`	`+ "total_gz": 11866`
`8`	`8`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`		`- "a.html": 12597,`
`3`		`- "a.html.gz": 6882,`
`4`		`- "total": 12597,`
`5`		`- "total_gz": 6882`
	`2`	`+ "a.html": 12517,`
	`3`	`+ "a.html.gz": 6871,`
	`4`	`+ "total": 12517,`
	`5`	`+ "total_gz": 6871`
`6`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"a.html": 17195,`
`3`		`- "a.html.gz": 7478,`
	`3`	`+ "a.html.gz": 7480,`
`4`	`4`	`"total": 17195,`
`5`		`- "total_gz": 7478`
	`5`	`+ "total_gz": 7480`
`6`	`6`	`}`