commit 59add4f01ef4741e10f64db9c2c8655cfe738ccb
author Ömer Ağacan <omersa@google.com> Tue Jul 02 01:32:55 2024 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Jul 02 01:32:55 2024 +0000
tree 42179c8ab12cfdec8ac64a5d961cae029483f9d8
parent 15f666c3eb496033d5028532beb6dd6da1cfedb8

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

This reverts commit 8b1aa1860f1c9ca0e59530576680a0b13b3f3400.

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>