OptimizeInstructions: Do not remove an inner cast when there are descriptor effects in the way (#9067)

Author: kripken

src/passes/OptimizeInstructions.cpp

@@ -2550,13 +2550,41 @@ struct OptimizeInstructions
       // traps are allowed, then we cannot remove the potentially-trapping
       // child, though.
       bool notWeaker = Type::isSubType(curr->type, child->type);
-      bool safe = !child->desc || getPassOptions().trapsNeverHappen;
-      if (notWeaker && safe) {
+      auto& options = getPassOptions();
+      auto canTrap = !options.trapsNeverHappen;
+      bool safe = !child->desc || !canTrap;
+      bool canOptimize = notWeaker && safe;
+      if (canOptimize && curr->desc && canTrap) {
+        // There is another child here, which might trap, and we need to
+        // consider that in this situation:
+        //
+        //  (outer.cast
+        //    (inner.cast (inner.ref))
+        //    (descriptor with effects)
+        //  )
+        //
+        //  =>
+        //
+        //  (outer.cast
+        //    (inner.ref)                ;; inner cast was removed
+        //    (descriptor with effects)
+        //  )
+        //
+        // It is safe to remove the inner cast, as if it trapped, the outer one
+        // would still trap. But if there is a descriptor, then we are moving
+        // the trap across the descriptor, and shouldn't cross effects there.
+        EffectAnalyzer descEffects(options, *getModule(), curr->desc);
+        ShallowEffectAnalyzer movingEffects(options, *getModule(), curr->ref);
+        if (movingEffects.orderedBefore(descEffects)) {
+          canOptimize = false;
+        }
+      }
+      if (canOptimize) {
         if (child->desc) {
           // Reorder the child's reference past its dropped descriptor if
           // necessary.
           auto* block =
-            ChildLocalizer(child, getFunction(), *getModule(), getPassOptions())
+            ChildLocalizer(child, getFunction(), *getModule(), options)
               .getChildrenReplacement();
           block->list.push_back(child->ref);
           block->type = child->ref->type;

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

@@ -1459,4 +1459,72 @@
       )
     )
   )
+
+  ;; CHECK:      (func $ref.cast_desc_eq-ref.cast (type $22) (param $x anyref)
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (ref.cast_desc_eq (ref $struct)
+  ;; CHECK-NEXT:    (ref.cast (ref $struct)
+  ;; CHECK-NEXT:     (local.get $x)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:    (block (result (ref null $desc))
+  ;; CHECK-NEXT:     (return)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (ref.cast_desc_eq (ref (exact $struct))
+  ;; CHECK-NEXT:    (local.get $x)
+  ;; CHECK-NEXT:    (struct.new_default $desc)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  ;; NTRAP:      (func $ref.cast_desc_eq-ref.cast (type $22) (param $x anyref)
+  ;; NTRAP-NEXT:  (local $1 (ref $struct))
+  ;; NTRAP-NEXT:  (local $2 (ref null $desc))
+  ;; NTRAP-NEXT:  (drop
+  ;; NTRAP-NEXT:   (block (result (ref $struct))
+  ;; NTRAP-NEXT:    (local.set $1
+  ;; NTRAP-NEXT:     (ref.cast (ref $struct)
+  ;; NTRAP-NEXT:      (local.get $x)
+  ;; NTRAP-NEXT:     )
+  ;; NTRAP-NEXT:    )
+  ;; NTRAP-NEXT:    (local.set $2
+  ;; NTRAP-NEXT:     (block (result (ref null $desc))
+  ;; NTRAP-NEXT:      (return)
+  ;; NTRAP-NEXT:     )
+  ;; NTRAP-NEXT:    )
+  ;; NTRAP-NEXT:    (local.get $1)
+  ;; NTRAP-NEXT:   )
+  ;; NTRAP-NEXT:  )
+  ;; NTRAP-NEXT:  (drop
+  ;; NTRAP-NEXT:   (ref.cast_desc_eq (ref (exact $struct))
+  ;; NTRAP-NEXT:    (local.get $x)
+  ;; NTRAP-NEXT:    (struct.new_default $desc)
+  ;; NTRAP-NEXT:   )
+  ;; NTRAP-NEXT:  )
+  ;; NTRAP-NEXT: )
+  (func $ref.cast_desc_eq-ref.cast (param $x anyref)
+    ;; As above with ref.as_non_null, removing the inner ref.cast would allow
+    ;; reaching the return before the cast check, so we do not optimize. (In
+    ;; NTRAP mode we end up removing the outer cast, separately.)
+    (drop
+      (ref.cast_desc_eq (ref $struct)
+        (ref.cast (ref $struct)
+          (local.get $x)
+        )
+        (block (result (ref null $desc))
+          (return)
+        )
+      )
+    )
+    ;; Without dangerous effects we can remove the inner cast.
+    (drop
+      (ref.cast_desc_eq (ref $struct)
+        (ref.cast (ref $struct)
+          (local.get $x)
+        )
+        (struct.new $desc)  ;; this has no effects
+      )
+    )
+  )
 )