From 9002cc6f87570afdcb000760f54abdad6861f1bd Mon Sep 17 00:00:00 2001
From: Thomas Lively <tlively@google.com>
Date: Wed, 13 Nov 2024 19:33:18 -0500
Subject: Fixup pops when necessary in IRBuilder (#7075)

IRBuilder introduces scratch locals to hoist values from underneath
stacky code to the top of the stack for consumption by the next
instruction. When it does so, the sequence of instructions from the set
to the get of the scratch local is packaged in a block so the entire
sequence can be made a child of the next instruction. In cases where the
hoisted value comes from a `pop`, this packaging can make the IR
invalid, since `pop`s are not allowed to appear inside blocks.

Detect when this problem might occur and fix it by running
`EHUtils::handleBlockNestedPops` after the function containing the
problem has been constructed.
---
 src/wasm/wasm-ir-builder.cpp | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

(limited to 'src/wasm/wasm-ir-builder.cpp')

diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp
index b238a926c..4cb9514f5 100644
--- a/src/wasm/wasm-ir-builder.cpp
+++ b/src/wasm/wasm-ir-builder.cpp
@@ -17,6 +17,7 @@
 #include <cassert>
 
 #include "ir/child-typer.h"
+#include "ir/eh-utils.h"
 #include "ir/names.h"
 #include "ir/properties.h"
 #include "ir/utils.h"
@@ -98,7 +99,10 @@ Result<> IRBuilder::packageHoistedValue(const HoistedVal& hoisted,
 
   auto packageAsBlock = [&](Type type) {
     // Create a block containing the producer of the hoisted value, the final
-    // get of the hoisted value, and everything in between.
+    // get of the hoisted value, and everything in between. Record the fact that
+    // we are synthesizing a block to help us determine later whether we need to
+    // run the nested pop fixup.
+    scopeStack[0].noteSyntheticBlock();
     std::vector<Expression*> exprs(scope.exprStack.begin() + hoisted.valIndex,
                                    scope.exprStack.end());
     auto* block = builder.makeBlock(exprs, type);
@@ -865,9 +869,12 @@ Result<> IRBuilder::visitCatch(Name tag) {
   tryy->catchTags.push_back(tag);
   pushScope(
     ScopeCtx::makeCatch(tryy, originalLabel, label, labelUsed, branchLabel));
-  // Push a pop for the exception payload.
+  // Push a pop for the exception payload if necessary.
   auto params = wasm.getTag(tag)->sig.params;
   if (params != Type::none) {
+    // Note that we have a pop to help determine later whether we need to run
+    // the fixup for pops within blocks.
+    scopeStack[0].notePop();
     push(builder.makePop(params));
   }
   return Ok{};
@@ -935,7 +942,7 @@ Result<> IRBuilder::visitEnd() {
   if (scope.isNone()) {
     return Err{"unexpected end"};
   }
-  if (auto* func = scope.getFunction(); func) {
+  if (auto* func = scope.getFunction()) {
     if (auto* loc = std::get_if<Function::DebugLocation>(&debugLoc)) {
       func->epilogLocation.insert(*loc);
     }
@@ -970,6 +977,9 @@ Result<> IRBuilder::visitEnd() {
   if (auto* func = scope.getFunction()) {
     func->body = maybeWrapForLabel(*expr);
     labelDepths.clear();
+    if (scope.needsPopFixup()) {
+      EHUtils::handleBlockNestedPops(func, wasm);
+    }
   } else if (auto* block = scope.getBlock()) {
     assert(*expr == block);
     block->name = scope.label;
-- 
cgit v1.2.3