OptimizeInstructions: Handle trivial ref.cast and ref.test

src/ir/localize.h

@@ -22,7 +22,10 @@
 namespace wasm {
 
 // Make an expression available in a local. If already in one, just
-// use that local, otherwise use a new local
+// use that local, otherwise use a new local.
+//
+// Note that if the local is reused, this assumes it is not modified in between
+// the set and the get, which the caller must ensure.
 
 struct Localizer {
   Index index;

src/ir/reorderer.h

@@ -0,0 +1,69 @@
+/*
+ * Copyright 2021 WebAssembly Community Group participants
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef wasm_ir_reorderer_h
+#define wasm_ir_reorderer_h
+
+#include <ir/effects.h>
+#include <wasm-builder.h>
+
+namespace wasm {
+
+//
+// Reorder two concrete expressions, using a local if necessary. Given
+//
+//  (first, second)
+//
+// we generate something so that it is valid to write
+//
+//  (Reorderer.second, Reorderer.first)
+//
+// (note the reversal of the order there). Reorderer.first/second will contain
+// either the original two expressions (first in first, second in second), or
+// ones with local usage to work around interactions between them.
+//
+
+struct Reorderer {
+  Expression* first;
+  Expression* second;
+
+  Reorderer(Expression* firstInput,
+            Expression* secondInput,
+            Function* func,
+            Module* wasm,
+            const PassOptions& passOptions) {
+    assert(firstInput->type.isConcrete());
+    assert(secondInput->type.isConcrete());
+
+    if (EffectAnalyzer::canReorder(
+          passOptions, wasm->features, firstInput, secondInput)) {
+      first = firstInput;
+      second = secondInput;
+      return;
+    }
+
+    auto type = firstInput->type;
+    auto index = Builder::addVar(func, type);
+    Builder builder(*wasm);
+    second = builder.makeSequence(builder.makeLocalSet(index, firstInput),
+                                  secondInput);
+    first = builder.makeLocalGet(index, type);
+  }
+};
+
+} // namespace wasm
+
+#endif // wasm_ir_reorderer_h

src/passes/OptimizeInstructions.cpp

@@ -34,6 +34,7 @@
 #include <ir/manipulation.h>
 #include <ir/match.h>
 #include <ir/properties.h>
+#include <ir/reorderer.h>
 #include <ir/type-updating.h>
 #include <ir/utils.h>
 #include <pass.h>
@@ -212,20 +213,33 @@ struct OptimizeInstructions
 
   bool fastMath;
 
+  bool refinalize = false;
+
   void doWalkFunction(Function* func) {
     fastMath = getPassOptions().fastMath;
-    // first, scan locals
+
+    // First, scan locals.
     {
       LocalScanner scanner(localInfo, getPassOptions());
       scanner.setModule(getModule());
       scanner.walkFunction(func);
     }
-    // main walk
+
+    // Main walk.
     super::doWalkFunction(func);
+
+    // If we need to update parent types, do so.
+    if (refinalize) {
+      ReFinalize().walkFunctionInModule(func, getModule());
+    }
+
+    // Final optimizations.
     {
       FinalOptimizer optimizer(getPassOptions());
       optimizer.walkFunction(func);
     }
+
+    TypeUpdating::handleNonDefaultableLocals(func, *getModule());
   }
 
   // Set to true when one of the visitors makes a change (either replacing the
@@ -1263,41 +1277,78 @@ struct OptimizeInstructions
     skipNonNullCast(curr->srcRef);
   }
 
+  bool canBeCastTo(HeapType a, HeapType b) {
+    return HeapType::isSubType(a, b) || HeapType::isSubType(b, a);
+  }
+
   void visitRefCast(RefCast* curr) {
     if (curr->type == Type::unreachable) {
       return;
     }
 
+    Builder builder(*getModule());
     auto passOptions = getPassOptions();
 
+    auto fallthrough = Properties::getFallthrough(
+      curr->ref, getPassOptions(), getModule()->features);
+
+    // If the value is a null, it will just flow through, and we do not need the
+    // cast. However, if that would change the type, then things are less
+    // simple: if the original type was non-nullable, replacing it with a null
+    // would change the type, which can happen in e.g.
+    //   (ref.cast (ref.as_non_null (.. (ref.null)
+    if (fallthrough->is<RefNull>()) {
+      Expression* rep =
+        builder.makeBlock({builder.makeDrop(curr->ref),
+                           builder.makeDrop(curr->rtt),
+                           builder.makeRefNull(curr->rtt->type.getHeapType())});
+      if (curr->ref->type.isNonNullable()) {
+        // Avoid a type change by forcing to be non-nullable. In practice, this
+        // would have trapped before we get here, so this is just for
+        // validation.
+        rep = builder.makeRefAs(RefAsNonNull, rep);
+      }
+      replaceCurrent(rep);
+      return;
+      // TODO: The optimal ordering of this and the other ref.as_non_null stuff
+      //       later down in this functions is unclear and may be worth looking
+      //       into.
+    }
+
+    // For the cast to be able to succeed, the value being cast must be a
+    // subtype of the desired type, as RTT subtyping is a subset of static
+    // subtyping. For example, trying to cast an array to a struct would be
+    // incompatible.
+    if (!canBeCastTo(curr->ref->type.getHeapType(),
+                     curr->rtt->type.getHeapType())) {
+      // This cast cannot succeed. It will either trap if the input is not a
+      // null, or it will return null if it is (and we have already handled the
+      // case of null before).
+      if (fallthrough->type.isNonNullable()) {
+        // Our type will now be unreachable; update the parents.
+        refinalize = true;
+        replaceCurrent(builder.makeBlock({builder.makeDrop(curr->ref),
+                                          builder.makeDrop(curr->rtt),
+                                          builder.makeUnreachable()}));
+        return;
+      }
+      // Otherwise, we are not sure what it is, and need to wait for runtime to
+      // see if it is a null or not.
+    }
+
     if (passOptions.ignoreImplicitTraps || passOptions.trapsNeverHappen) {
-      // A ref.cast traps when the RTTs do not line up, which can be of one of
-      // two sorts of issues:
-      //  1. The value being cast is not even a subtype of the cast type. In
-      //     that case the RTTs trivially cannot indicate subtyping, because
-      //     RTT subtyping is a subset of static subtyping. For example, maybe
-      //     we are trying to cast a {i32} struct to an [f64] array.
-      //  2. The value is a subtype of the cast type, but the RTTs still do not
-      //     fit. That indicates a difference between RTT subtyping and static
-      //     subtyping. That is, the type may be right but the chain of rtt.subs
-      //     is not.
-      // If we ignore a possible trap then we would like to assume that neither
-      // of those two situations can happen. However, we still cannot do
-      // anything if point 1 is a problem, that is, if the value is not a
-      // subtype of the cast type, as we can't remove the cast in that case -
-      // the wasm would not validate. But if the type *is* a subtype, then we
-      // can ignore a possible trap on 2 and remove it.
-      //
-      // We also do not do this if the arguments cannot be reordered. If we
-      // can't do that then we need to add a drop, at minimum (which may still
-      // be worthwhile, but depends on other optimizations kicking in, so it's
-      // not clearly worthwhile).
+      // Aside from the issue of type incompatibility as mentioned above, the
+      // cast can trap if the types *are* compatible but it happens to be the
+      // case at runtime that the value is not of the desired subtype. If we
+      // do not consider such traps possible, we can ignore that. Note, though,
+      // that we cannot do this if we cannot replace the current type with the
+      // reference's type.
       if (HeapType::isSubType(curr->ref->type.getHeapType(),
-                              curr->rtt->type.getHeapType()) &&
-          canReorder(curr->ref, curr->rtt)) {
-        Builder builder(*getModule());
-        replaceCurrent(
-          builder.makeSequence(builder.makeDrop(curr->rtt), curr->ref));
+                              curr->rtt->type.getHeapType())) {
+        Reorderer reorderer(
+          curr->ref, curr->rtt, getFunction(), getModule(), passOptions);
+        replaceCurrent(builder.makeSequence(builder.makeDrop(reorderer.second),
+                                            reorderer.first));
         return;
       }
     }
@@ -1358,6 +1409,18 @@ struct OptimizeInstructions
     }
   }
 
+  void visitRefTest(RefTest* curr) {
+    // See above in RefCast.
+    if (!canBeCastTo(curr->ref->type.getHeapType(),
+                     curr->rtt->type.getHeapType())) {
+      // This test cannot succeed, and will definitely return 0.
+      Builder builder(*getModule());
+      replaceCurrent(builder.makeBlock({builder.makeDrop(curr->ref),
+                                        builder.makeDrop(curr->rtt),
+                                        builder.makeConst(int32_t(0))}));
+    }
+  }
+
   void visitRefIs(RefIs* curr) {
     if (curr->type == Type::unreachable) {
       return;

test/lit/passes/inlining-optimizing.wast

@@ -4,7 +4,6 @@
 (module
  ;; CHECK:      (type $none_=>_none (func))
  (type $none_=>_none (func))
- ;; CHECK:      (type $none_=>_i32 (func (result i32)))
  (type $none_=>_i32 (func (result i32)))
  ;; CHECK:      (func $0
  ;; CHECK-NEXT:  (nop)
@@ -15,10 +14,7 @@
  ;; CHECK:      (func $1
  ;; CHECK-NEXT:  (drop
  ;; CHECK-NEXT:   (call_ref
- ;; CHECK-NEXT:    (ref.cast
- ;; CHECK-NEXT:     (ref.func $0)
- ;; CHECK-NEXT:     (rtt.canon $none_=>_i32)
- ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:    (unreachable)
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )

test/lit/passes/optimize-instructions-call_ref.wast

@@ -143,9 +143,14 @@
 
  ;; CHECK:      (func $fallthrough-bad-type (result i32)
  ;; CHECK-NEXT:  (call_ref
- ;; CHECK-NEXT:   (ref.cast
- ;; CHECK-NEXT:    (ref.func $return-nothing)
- ;; CHECK-NEXT:    (rtt.canon $none_=>_i32)
+ ;; CHECK-NEXT:   (block
+ ;; CHECK-NEXT:    (drop
+ ;; CHECK-NEXT:     (ref.func $return-nothing)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:    (drop
+ ;; CHECK-NEXT:     (rtt.canon $none_=>_i32)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:    (unreachable)
  ;; CHECK-NEXT:   )
  ;; CHECK-NEXT:  )
  ;; CHECK-NEXT: )