languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-14 16:26:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "[dart2wasm] Use single unsigned cmp instead of two cmps when possible"

Browse source 
This reverts commit 8b1aa1860f.

Reason for revert:

This caused at least two issues:

(1) wasm-opt doesn't inline the simple helpers introduced in this CL,
for example:

```
  (func $IntToWasmInt|geU (;217;) (param $var0 i64) (param $var1 i64) (result i32)
    local.get $var0
    local.get $var1
    i64.ge_u
  )
```

Even though when inlined this becomes just one instruction.

This causes diffs like:

```
               i64.const 1
               i64.add
               local.set $var1
+              local.get $var2
               local.get $var10
               struct.get $JSArrayBufferImpl_80 $field2
               call $wasm:js-string.length (import)
               i64.extend_i32_u
               local.tee $var3
-              local.get $var2
-              i64.le_u
+              call $IntToWasmInt|geU
               if
```

(2) Changing `length.leU(index)` to `index.geU(length)` causes missing
some optimizations like the following:

```
     local.get $var0
     ref.cast $JSArrayBufferImpl_80
     local.tee $var3
     struct.get $JSArrayBufferImpl_80 $field2
     call $wasm:js-string.length (import)
-    drop
+    i64.extend_i32_u
+    local.tee $var4
+    i64.const 0
+    i64.lt_u
+    if
+      i64.const 0
+      i64.const 0
+      local.get $var4
+      ref.null none
+      ref.null none
+      call $RangeError.range
+      call $Error._throwWithCurrentStackTrace
+      unreachable
+    end
```

Here the `i64.const 0; i64.lt_u` always produces `0`, but wasm-opt
doesn't do this optimization.

The Dart changes that caused this diff:

```
-    if (WasmI64.fromInt(length).leU(WasmI64.fromInt(index))) {
+    if (index.geU(length)) {
```

Original change's description:
> [dart2wasm] Use single unsigned cmp instead of two cmps when possible
>
> wasm-opt doesn't optimize `0 < x || x > y` when y is known to be
> positive (e.g. a positive integer constant), so we do it manually.
>
> We also do it in a few places where `y` is not known to be positive in
> the Wasm code, but we know it's always positive, for example when it's a
> length.
>
> Example improvement in the wasm-opt output:
>
> ```
>    (func $_newArrayLengthCheck (;426;) (param $var0 i64) (result i64)
>      local.get $var0
>      i64.const 2147483647
> -    i64.le_s
> -    local.get $var0
> -    i64.const 0
> -    i64.ge_s
> -    i32.and
> -    i32.eqz
> +    i64.gt_u
>      if
>        i32.const 46
>        i32.const 0
> @@ -19190,13 +19172,8 @@
>                i64.const 97
>                i64.sub
>                local.tee $var3
> -              i64.const 0
> -              i64.ge_s
> -              local.get $var3
>                i64.const 5
> -              i64.le_s
> -              i32.and
> -              i32.eqz
> +              i64.gt_u
>                if
>                  local.get $var0
>                  local.get $var6
> @@ -19810,10 +19787,10 @@
>      global.get $global4
>      array.new_fixed $Array<_Type> 2
>    )
> ```
>
> Closes #56083.
>
> Change-Id: Idb1dd0d0809b26be8aec3d082aa341c59e1a353d
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373663
> Reviewed-by: Martin Kustermann <kustermann@google.com>
> Commit-Queue: Ömer Ağacan <omersa@google.com>

Change-Id: Iac5428037b0c19d76e4c841a1b6623b7305ff702
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373800
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
This commit is contained in:
Ömer Ağacan 2024-07-02 01:32:55 +00:00 committed by Commit Queue
parent 15f666c3eb
commit 59add4f01e
12 changed files with 35 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
pkg/dart2wasm/lib/intrinsics.dart
View file

@ -250,16 +250,6 @@ class Intrinsifier {
codeGen.wrap(node.arguments.positional[0], w.NumType.i64);
b.i64_lt_u();
return boolType;
case "geU":
codeGen.wrap(receiver, w.NumType.i64);
codeGen.wrap(node.arguments.positional[0], w.NumType.i64);
b.i64_ge_u();
return boolType;
case "gtU":
codeGen.wrap(receiver, w.NumType.i64);
codeGen.wrap(node.arguments.positional[0], w.NumType.i64);
b.i64_gt_u();
return boolType;
default:
throw 'Unknown WasmI64 member $name';
}

6
sdk/lib/_internal/wasm/lib/boxed_double.dart
View file

@ -348,8 +348,7 @@ final class _BoxedDouble extends double {
// See ECMAScript-262, 15.7.4.5 for details.
// Step 2.
// fractionDigits < 0 || fractionDigits > 20
if (fractionDigits.gtU(20)) {
if (fractionDigits < 0 || fractionDigits > 20) {
throw new RangeError.range(fractionDigits, 0, 20, "fractionDigits");
}
@ -388,8 +387,7 @@ final class _BoxedDouble extends double {
// Step 7.
if (fractionDigits != null) {
// fractionDigits < 0 || fractionDigits > 20
if (fractionDigits.gtU(20)) {
if (fractionDigits < 0 || fractionDigits > 20) {
throw new RangeError.range(fractionDigits, 0, 20, "fractionDigits");
}
}

7
sdk/lib/_internal/wasm/lib/boxed_int.dart
View file

@ -3,7 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
import 'dart:_internal';
import 'dart:_wasm';
@pragma("wasm:entry-point")
final class _BoxedInt extends int {
@ -300,8 +299,7 @@ final class _BoxedInt extends int {
}
int b = this;
// b < 0 || b > m, m is positive (checked above)
if (b.gtU(m)) {
if (b < 0 || b > m) {
b %= m;
}
int r = 1;
@ -392,8 +390,7 @@ final class _BoxedInt extends int {
if (m <= 0) throw new RangeError.range(m, 1, null, "modulus");
if (m == 1) return 0;
int t = this;
// t < 0 || t >= m, m is positive (checked above)
if (t.geU(m)) t %= m;
if ((t < 0) || (t >= m)) t %= m;
if (t == 1) return 1;
if ((t == 0) || (t.isEven && m.isEven)) {
throw new Exception("Not coprime");

6
sdk/lib/_internal/wasm/lib/convert_patch.dart
View file

@ -1221,8 +1221,7 @@ mixin _ChunkedJsonParser<T> on _JsonParserWithListener {
value += digit;
} else {
digit = (char | 0x20) - CHAR_a;
// digit < 0 || digit > 5
if (digit.gtU(5)) {
if (digit < 0 || digit > 5) {
fail(hexStart, "Invalid unicode escape");
}
value += digit + 10;
@ -1807,8 +1806,7 @@ class _Utf8Decoder {
int u8listIdx = 0;
for (int codeUnitsIdx = start; codeUnitsIdx < end; codeUnitsIdx += 1) {
int byte = codeUnits[codeUnitsIdx];
// byte < 0 || byte > 255
if (byte.gtU(255)) {
if (byte < 0 || byte > 255) {
if (allowMalformed) {
byte = 0xFF;
} else {

8
sdk/lib/_internal/wasm/lib/error_utils.dart
View file

@ -9,7 +9,7 @@ class IndexErrorUtils {
/// uses a single unsigned comparison. Always inlined.
@pragma("wasm:prefer-inline")
static void checkAssumePositiveLength(int index, int length) {
if (index.geU(length)) {
if (WasmI64.fromInt(length).leU(WasmI64.fromInt(index))) {
throw IndexError.withLength(index, length);
}
}
@ -21,7 +21,7 @@ class RangeErrorUtils {
/// comparison. Always inlined.
@pragma("wasm:prefer-inline")
static void checkValueBetweenZeroAndPositiveMax(int value, int maxValue) {
if (value.gtU(maxValue)) {
if (WasmI64.fromInt(maxValue).ltU(WasmI64.fromInt(value))) {
throw RangeError.range(value, 0, maxValue);
}
}
@ -32,10 +32,10 @@ class RangeErrorUtils {
/// instead of [start]. Always inlined.
@pragma("wasm:prefer-inline")
static void checkValidRangePositiveLength(int start, int end, int length) {
if (end.gtU(length)) {
if (WasmI64.fromInt(length).ltU(WasmI64.fromInt(end))) {
throw RangeError.range(end, 0, length);
}
if (start.gtU(end)) {
if (WasmI64.fromInt(end).ltU(WasmI64.fromInt(start))) {
throw RangeError.range(start, 0, end);
}
}

4
sdk/lib/_internal/wasm/lib/internal_patch.dart
View file

@ -171,7 +171,7 @@ external bool get _checkBounds;
/// Assumes that [length] is positive.
@pragma("wasm:prefer-inline")
void indexCheck(int index, int length) {
if (_checkBounds && index.geU(length)) {
if (_checkBounds && WasmI64.fromInt(length).leU(WasmI64.fromInt(index))) {
throw IndexError.withLength(index, length);
}
}
@ -179,7 +179,7 @@ void indexCheck(int index, int length) {
/// Same as [indexCheck], but passes [name] to [IndexError].
@pragma("wasm:prefer-inline")
void indexCheckWithName(int index, int length, String name) {
if (_checkBounds && index.geU(length)) {
if (_checkBounds && WasmI64.fromInt(length).leU(WasmI64.fromInt(index))) {
throw IndexError.withLength(index, length, name: name);
}
}

6
sdk/lib/_internal/wasm/lib/list.dart
View file

@ -317,8 +317,7 @@ class GrowableList<E> extends _ModifiableList<E> {
return add(element);
}
// index < 0 || index > length
if (index.gtU(length)) {
if ((index < 0) || (index > length)) {
throw RangeError.range(index, 0, length);
}
@ -368,8 +367,7 @@ class GrowableList<E> extends _ModifiableList<E> {
}
void insertAll(int index, Iterable<E> iterable) {
// index < 0 || index > length
if (index.gtU(length)) {
if (index < 0 || index > length) {
throw RangeError.range(index, 0, length);
}
if (iterable is! WasmListBase) {

9
sdk/lib/_internal/wasm/lib/regexp_helper.dart
View file

@ -136,8 +136,7 @@ class JSSyntaxRegExp implements RegExp {
}
Iterable<RegExpMatch> allMatches(String string, [int start = 0]) {
// start < 0 || start > string.length
if (start.gtU(string.length)) {
if (start < 0 || start > string.length) {
throw new RangeError.range(start, 0, string.length);
}
return _AllMatchesIterable(this, string, start);
@ -162,8 +161,7 @@ class JSSyntaxRegExp implements RegExp {
}
RegExpMatch? matchAsPrefix(String string, [int start = 0]) {
// start < 0 || start > string.length
if (start.gtU(string.length)) {
if (start < 0 || start > string.length) {
throw new RangeError.range(start, 0, string.length);
}
return _execAnchored(string, start);
@ -188,8 +186,7 @@ class _MatchImplementation implements RegExpMatch {
int get end => (start + (_match[0.toJS].toString()).length);
String? group(int index) {
// index < 0 || index >= _match.length.toDartInt
if (index.geU(_match.length.toDartInt)) {
if (index < 0 || index >= _match.length.toDartInt) {
throw RangeError("Index $index is out of range ${_match.length}");
}
return _match[index.toJS]?.toString();

13
sdk/lib/_internal/wasm/lib/simd.dart
View file

@ -10,7 +10,6 @@ import 'dart:collection' show ListMixin;
import 'dart:math' as math;
import 'dart:typed_data';
import 'dart:_internal' show WasmTypedDataBase;
import 'dart:_wasm';
final class NaiveInt32x4List extends WasmTypedDataBase
with ListMixin<Int32x4>, FixedLengthListMixin<Int32x4>
@ -510,8 +509,7 @@ final class NaiveFloat32x4 extends WasmTypedDataBase implements Float32x4 {
}
Float32x4 shuffle(int mask) {
// mask < 0 || mask > 255
if (mask.gtU(255)) {
if ((mask < 0) || (mask > 255)) {
throw RangeError.range(mask, 0, 255, 'mask');
}
_list[0] = x;
@ -527,8 +525,7 @@ final class NaiveFloat32x4 extends WasmTypedDataBase implements Float32x4 {
}
Float32x4 shuffleMix(Float32x4 other, int mask) {
// mask < 0 || mask > 255
if (mask.gtU(255)) {
if ((mask < 0) || (mask > 255)) {
throw RangeError.range(mask, 0, 255, 'mask');
}
_list[0] = x;
@ -775,8 +772,7 @@ final class NaiveInt32x4 extends WasmTypedDataBase implements Int32x4 {
}
Int32x4 shuffle(int mask) {
// mask < 0 || mask > 255
if (mask.gtU(255)) {
if ((mask < 0) || (mask > 255)) {
throw RangeError.range(mask, 0, 255, 'mask');
}
_list[0] = x;
@ -791,8 +787,7 @@ final class NaiveInt32x4 extends WasmTypedDataBase implements Int32x4 {
}
Int32x4 shuffleMix(Int32x4 other, int mask) {
// mask < 0 || mask > 255
if (mask.gtU(255)) {
if ((mask < 0) || (mask > 255)) {
throw RangeError.range(mask, 0, 255, 'mask');
}
_list[0] = x;

30
sdk/lib/_internal/wasm/lib/string.dart
View file

@ -220,8 +220,7 @@ abstract final class StringBase extends WasmStringBase
bits |= nonBmpCode | 0x10000;
multiCodeUnitChars += 1;
}
// bits < 0 || bits > 0xFFFFF
if (bits.gtU(0xFFFFF)) {
if (bits < 0 || bits > 0xFFFFF) {
throw ArgumentError(typedCharCodes);
}
if (multiCodeUnitChars == 0) {
@ -277,8 +276,7 @@ abstract final class StringBase extends WasmStringBase
..add(0xDC00 | (nonBmpChar & 0x3FF));
}
}
// bits < 0 || bits > 0xFFFFF
if (bits.gtU(0xFFFFF)) {
if (bits < 0 || bits > 0xFFFFF) {
throw ArgumentError(charCodes);
}
List<int> charCodeList = makeListFixedLength<int>(list);
@ -396,8 +394,7 @@ abstract final class StringBase extends WasmStringBase
}
bool startsWith(Pattern pattern, [int index = 0]) {
// index < 0 || index > length
if (index.gtU(length)) {
if ((index < 0) || (index > this.length)) {
throw RangeError.range(index, 0, this.length);
}
if (pattern is String) {
@ -407,8 +404,7 @@ abstract final class StringBase extends WasmStringBase
}
int indexOf(Pattern pattern, [int start = 0]) {
// start < 0 || start > length
if (start.gtU(length)) {
if ((start < 0) || (start > this.length)) {
throw RangeError.range(start, 0, this.length, "start");
}
if (pattern is String) {
@ -433,8 +429,7 @@ abstract final class StringBase extends WasmStringBase
int lastIndexOf(Pattern pattern, [int? start]) {
if (start == null) {
start = this.length;
} else if (start.gtU(length)) {
// start < 0 || start > length
} else if (start < 0 || start > this.length) {
throw RangeError.range(start, 0, this.length);
}
if (pattern is String) {
@ -628,9 +623,8 @@ abstract final class StringBase extends WasmStringBase
bool contains(Pattern pattern, [int startIndex = 0]) {
if (pattern is String) {
// startIndex < 0 || startIndex > length
if (startIndex.gtU(length)) {
throw RangeError.range(startIndex, 0, length);
if (startIndex < 0 || startIndex > this.length) {
throw RangeError.range(startIndex, 0, this.length);
}
return indexOf(pattern, startIndex) >= 0;
}
@ -1016,16 +1010,14 @@ abstract final class StringBase extends WasmStringBase
}
Iterable<Match> allMatches(String string, [int start = 0]) {
// start < 0 || start > string.length
if (start.gtU(string.length)) {
if (start < 0 || start > string.length) {
throw RangeError.range(start, 0, string.length, "start");
}
return StringAllMatchesIterable(string, this, start);
}
Match? matchAsPrefix(String string, [int start = 0]) {
// start < 0 || start > string.length
if (start.gtU(string.length)) {
if (start < 0 || start > string.length) {
throw RangeError.range(start, 0, string.length);
}
if (start + this.length > string.length) return null;
@ -1188,7 +1180,7 @@ final class OneByteString extends StringBase {
@override
int codeUnitAt(int index) {
if (index.geU(length)) {
if (WasmI64.fromInt(length).leU(WasmI64.fromInt(index))) {
throw IndexError.withLength(index, length);
}
return _codeUnitAtUnchecked(index);
@ -1619,7 +1611,7 @@ final class TwoByteString extends StringBase {
@override
int codeUnitAt(int index) {
if (index.geU(length)) {
if (WasmI64.fromInt(length).leU(WasmI64.fromInt(index))) {
throw IndexError.withLength(index, length);
}
return _codeUnitAtUnchecked(index);

3
sdk/lib/_internal/wasm/lib/typed_data.dart
View file

@ -36,8 +36,7 @@ import 'dart:typed_data';
const int _maxWasmArrayLength = 2147483647; // max i32
int _newArrayLengthCheck(int length) {
// length < 0 || length > _maxWasmArrayLength
if (length.gtU(_maxWasmArrayLength)) {
if (length < 0 || length > _maxWasmArrayLength) {
throw RangeError.value(length);
}
return length;

16
sdk/lib/_wasm/wasm_types.dart
View file

@ -146,17 +146,11 @@ class WasmI64 extends _WasmBase {
external int toInt();
/// Wasm `i64.le_u` instruction.
/// `i64.le_u`.
external bool leU(WasmI64 other);
/// Wasm `i64.lt_u` instruction.
/// `i64.lt_u`.
external bool ltU(WasmI64 other);
/// Wasm `i64.ge_u` instruction.
external bool geU(WasmI64 other);
/// Wasm `i64.gt_u` instruction.
external bool gtU(WasmI64 other);
}
/// The Wasm `f32` type.
@ -297,12 +291,6 @@ class WasmTable<T> extends _WasmBase {
extension IntToWasmInt on int {
WasmI32 toWasmI32() => WasmI32.fromInt(this);
WasmI64 toWasmI64() => WasmI64.fromInt(this);
/// Wasm `i64.ge_u` instruction.
bool geU(int other) => this.toWasmI64().geU(other.toWasmI64());
/// Wasm `i64.gt_u` instruction.
bool gtU(int other) => this.toWasmI64().gtU(other.toWasmI64());
}
extension DoubleToWasmFloat on double {