From 43ed681debe7928c894f2ae57823f08988d23475 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Wed, 11 Sep 2024 17:08:24 -0700
Subject: [PATCH] [NFC] Ignore invalid precomputed fallthrough values  (#6933)

Followup to #6928. If the fallthrough precomputes to an invalid type, it
makes no sense to precompute the non-fallthrough, as it can't return anything
useful.
---
 src/passes/Precompute.cpp | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp
index 379f6ee49c6..0b8ead056a8 100644
--- a/src/passes/Precompute.cpp
+++ b/src/passes/Precompute.cpp
@@ -787,26 +787,20 @@ struct Precompute
       auto values = precomputeValue(
         Properties::getFallthrough(set->value, getPassOptions(), *getModule()));
 
-      // Fix up the value. The computation we just did was to look at the
-      // fallthrough, then precompute that; that looks through expressions
-      // that pass through the value. Normally that does not matter here,
-      // for example, (block .. (value)) returns the value unmodified.
-      // However, some things change the type, for example RefAsNonNull has
-      // a non-null type, while its input may be nullable. That does not
-      // matter either, as if we managed to precompute it then the value had
-      // the more specific (in this example, non-nullable) type. But there
-      // is a situation where this can cause an issue: RefCast. An attempt to
-      // perform a "bad" cast, say of a null to non-null, is a tricky case where
-      // the fallthrough value's type is very different than the actually
-      // returned value's type. To handle that, if we precomputed a value and
-      // if it has the wrong type then precompute it again without looking
-      // through to the fallthrough.
+      // We precomputed the *fallthrough* value (which allows us to look through
+      // some things that would otherwise block us). But in some cases, like a
+      // ref.cast, the fallthrough value can have an incompatible type for the
+      // entire expression, which would be invalid for us to propagate, e.g.:
+      //
+      //  (ref.cast (ref struct)
+      //    (ref.null any)
+      //  )
+      //
+      // In such a case the value cannot actually fall through. Ignore such
+      // cases (which other optimizations can handle) by making sure that we
+      // only propagate a valid subtype.
       if (values.isConcrete() &&
-          !Type::isSubType(values.getType(), set->value->type)) {
-        values = precomputeValue(set->value);
-      }
-
-      if (values.isConcrete()) {
+          Type::isSubType(values.getType(), set->value->type)) {
         setValues[set] = values;
         work.push_back(set);
       }