Skip to content

[MLIR][Wasm] Introduce the WasmSSA MLIR dialect #149233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

flemairen6
Copy link

@flemairen6 flemairen6 commented Jul 17, 2025

Introduce the WasmSSA dialect as discussed in https://discourse.llvm.org/t/rfc-mlir-dialect-for-webassembly/86758 and during the ODM https://discourse.llvm.org/t/mlir-open-meeting-webassembly-dialect/86928

This is the first PR to introduce the WasmSSA dialect (it has been renamed to clarify that it doesn't represent the stack based semantic of Webassembly, and to allow interfacing with a future Wasm dialect that would model this stack based approach).

This PR only introduces the dialect definition and interfaces, the list of operators and some operators-related helper functions (related to parsing or verification) and some tests for those.
Follow-up PRs will bring the binary Webassembly importer and the lowerings to other dialects along with testing and a driver for conversion of Webassembly binaries to LLVM IR.

Fyi: @ftynse @jpienaar

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added the mlir label Jul 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 17, 2025

@llvm/pr-subscribers-mlir

Author: Ferdinand Lemaire (flemairen6)

Changes

Introduce the WasmSSA dialect as discussed in https://discourse.llvm.org/t/rfc-mlir-dialect-for-webassembly/86758 and during the ODM https://discourse.llvm.org/t/mlir-open-meeting-webassembly-dialect/86928

This is the first PR to introduce the WasmSSA dialect (it has been renamed to clarify that it doesn't represent the stack based semantic of Webassembly, and to allow interfacing with a future Wasm dialect that would model this stack based approach).

This PR only introduces the dialect definition and interfaces, the list of operators and some operators-related helper functions (related to parsing or verification) and some tests for those.
Follow-up PRs will bring the binary Webassembly importer and the lowerings to other dialects along with testing and a driver for conversion of Webassembly binaries to LLVM IR.


Patch is 79.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/149233.diff

23 Files Affected:

  • (modified) .github/CODEOWNERS (+3)
  • (modified) mlir/include/mlir/Dialect/CMakeLists.txt (+1)
  • (added) mlir/include/mlir/Dialect/WebAssemblySSA/CMakeLists.txt (+1)
  • (added) mlir/include/mlir/Dialect/WebAssemblySSA/IR/CMakeLists.txt (+13)
  • (added) mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSA.h (+59)
  • (added) mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSABase.td (+25)
  • (added) mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.h (+30)
  • (added) mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.td (+186)
  • (added) mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAOps.td (+679)
  • (added) mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSATypes.td (+86)
  • (modified) mlir/include/mlir/InitAllDialects.h (+2)
  • (modified) mlir/lib/Dialect/CMakeLists.txt (+1)
  • (added) mlir/lib/Dialect/WebAssemblySSA/CMakeLists.txt (+1)
  • (added) mlir/lib/Dialect/WebAssemblySSA/IR/CMakeLists.txt (+24)
  • (added) mlir/lib/Dialect/WebAssemblySSA/IR/WebAssemblySSADialect.cpp (+41)
  • (added) mlir/lib/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.cpp (+62)
  • (added) mlir/lib/Dialect/WebAssemblySSA/IR/WebAssemblySSAOps.cpp (+513)
  • (added) mlir/lib/Dialect/WebAssemblySSA/IR/WebAssemblySSATypes.cpp (+39)
  • (added) mlir/test/Dialect/WebAssemblySSA/custom_parser/global.mlir (+44)
  • (added) mlir/test/Dialect/WebAssemblySSA/custom_parser/global_illegal.mlir (+11)
  • (added) mlir/test/Dialect/WebAssemblySSA/custom_parser/global_illegal_2.mlir (+11)
  • (added) mlir/test/Dialect/WebAssemblySSA/custom_parser/import.mlir (+17)
  • (added) mlir/test/Dialect/WebAssemblySSA/custom_parser/local.mlir (+45)
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
index dd7e1771c9361..8bb01bbc4a97e 100644
--- a/.github/CODEOWNERS
+++ b/.github/CODEOWNERS
@@ -105,6 +105,9 @@
 /mlir/**/*ToSPIRV/ @antiagainst @kuhar
 /mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp @antiagainst @kuhar
 
+# WASM Dialect in MLIR.
+/mlir/**/WebAssemblySSA/ @flemairen6 @lforg37 @ornata 
+
 # MLIR Sparsifier.
 /mlir/**/*SparseTensor*/ @aartbik @PeimingLiu @yinying-lisa-li @matthias-springer
 
diff --git a/mlir/include/mlir/Dialect/CMakeLists.txt b/mlir/include/mlir/Dialect/CMakeLists.txt
index 56dc97282fa4a..eb6075ac76c85 100644
--- a/mlir/include/mlir/Dialect/CMakeLists.txt
+++ b/mlir/include/mlir/Dialect/CMakeLists.txt
@@ -41,5 +41,6 @@ add_subdirectory(Transform)
 add_subdirectory(UB)
 add_subdirectory(Utils)
 add_subdirectory(Vector)
+add_subdirectory(WebAssemblySSA)
 add_subdirectory(X86Vector)
 add_subdirectory(XeGPU)
diff --git a/mlir/include/mlir/Dialect/WebAssemblySSA/CMakeLists.txt b/mlir/include/mlir/Dialect/WebAssemblySSA/CMakeLists.txt
new file mode 100644
index 0000000000000..f33061b2d87cf
--- /dev/null
+++ b/mlir/include/mlir/Dialect/WebAssemblySSA/CMakeLists.txt
@@ -0,0 +1 @@
+add_subdirectory(IR)
diff --git a/mlir/include/mlir/Dialect/WebAssemblySSA/IR/CMakeLists.txt b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/CMakeLists.txt
new file mode 100644
index 0000000000000..fa41a0caabf91
--- /dev/null
+++ b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_TARGET_DEFINITIONS WebAssemblySSATypes.td)
+mlir_tablegen(WebAssemblySSATypeConstraints.h.inc -gen-type-constraint-decls)
+mlir_tablegen(WebAssemblySSATypeConstraints.cpp.inc -gen-type-constraint-defs)
+
+set (LLVM_TARGET_DEFINITIONS WebAssemblySSAInterfaces.td)
+mlir_tablegen(WebAssemblySSAInterfaces.h.inc -gen-op-interface-decls)
+mlir_tablegen(WebAssemblySSAInterfaces.cpp.inc -gen-op-interface-defs)
+add_public_tablegen_target(MLIRWebAssemblySSAInterfacesIncGen)
+
+set(LLVM_TARGET_DEFINITIONS WebAssemblySSAOps.td)
+
+add_mlir_dialect(WebAssemblySSAOps wasmssa)
+add_mlir_doc(WebAssemblySSAOps WebAssemblySSAOps Dialects/ -gen-dialect-doc)
diff --git a/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSA.h b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSA.h
new file mode 100644
index 0000000000000..6636378651a97
--- /dev/null
+++ b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSA.h
@@ -0,0 +1,59 @@
+//===- WebAssemblySSA.h - WebAssemblySSA dialect ------------------*- C++-*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_DIALECT_WEBASSEMBLYSSA_IR_WEBASSEMBLYSSA_H_
+#define MLIR_DIALECT_WEBASSEMBLYSSA_IR_WEBASSEMBLYSSA_H_
+
+
+#include "mlir/Bytecode/BytecodeOpInterface.h"
+#include "mlir/IR/Dialect.h"
+
+//===----------------------------------------------------------------------===//
+// WebAssemblyDialect
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAOpsDialect.h.inc"
+
+
+//===----------------------------------------------------------------------===//
+// WebAssembly Dialect Types
+//===----------------------------------------------------------------------===//
+
+#define GET_TYPEDEF_CLASSES
+#include "mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAOpsTypes.h.inc"
+
+
+//===----------------------------------------------------------------------===//
+// WebAssembly Interfaces
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.h"
+
+//===----------------------------------------------------------------------===//
+// WebAssembly Dialect Operations
+//===----------------------------------------------------------------------===//
+#include "mlir/Interfaces/CallInterfaces.h"
+#include "mlir/Interfaces/FunctionInterfaces.h"
+#include "mlir/Interfaces/InferTypeOpInterface.h"
+#include "mlir/IR/SymbolTable.h"
+
+
+//===----------------------------------------------------------------------===//
+// WebAssembly Constraints
+//===----------------------------------------------------------------------===//
+
+namespace mlir{
+namespace wasmssa {
+#include "mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSATypeConstraints.h.inc"
+}
+}// namespace mlir
+
+#define GET_OP_CLASSES
+#include "mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAOps.h.inc"
+
+#endif // MLIR_DIALECT_WEBASSEMBLYSSA_IR_WEBASSEMBLYSSA_H_
diff --git a/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSABase.td b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSABase.td
new file mode 100644
index 0000000000000..a4a98a25dae0c
--- /dev/null
+++ b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSABase.td
@@ -0,0 +1,25 @@
+//===- WebAssemblySSABase.td - Base defs for wasmssa dialect -*- tablegen -*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef WEBASSEMBLYSSA_BASE
+#define WEBASSEMBLYSSA_BASE
+
+include "mlir/IR/EnumAttr.td"
+include "mlir/IR/OpBase.td"
+
+def WasmSSA_Dialect : Dialect {
+  let name = "wasmssa";
+  let cppNamespace = "::mlir::wasmssa";
+  let description = [{
+    The wasmssa dialect is intended to represent WebAssembly
+    modules in ssa form for easier manipulation.
+  }];
+  let useDefaultTypePrinterParser = true;
+}
+
+#endif //WEBASSEMBLYSSA_BASE
diff --git a/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.h b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.h
new file mode 100644
index 0000000000000..efc2af02d4f73
--- /dev/null
+++ b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.h
@@ -0,0 +1,30 @@
+//===- WebAssemblySSAInterfaces.h - WebAssemblySSA Interfaces ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines op interfaces for the WebAssemblySSA dialect in MLIR.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_DIALECT_WEBASSEMBLYSSA_IR_WEBASSEMBLYSSAINTERFACES_H_
+#define MLIR_DIALECT_WEBASSEMBLYSSA_IR_WEBASSEMBLYSSAINTERFACES_H_
+
+#include "mlir/IR/BuiltinAttributes.h"
+#include "mlir/IR/OpDefinition.h"
+
+namespace mlir {
+namespace wasmssa {
+namespace detail {
+    LogicalResult verifyConstantExpressionInterface(Operation *op);
+    LogicalResult verifyWasmSSALabelBranchingInterface(Operation *op);
+} // namespace detail
+
+#include "mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.h.inc"
+} // namespace wasmssa
+} // namespace mlir
+
+#endif // MLIR_DIALECT_WEBASSEMBLYSSA_IR_WEBASSEMBLYSSAINTERFACES_H_
diff --git a/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.td b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.td
new file mode 100644
index 0000000000000..7857556871c3f
--- /dev/null
+++ b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.td
@@ -0,0 +1,186 @@
+//===-- WebAssemblySSAInterfaces.td - WebAssemblySSA Interfaces -*- tablegen -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines interfaces for the WebAssemblySSA dialect in MLIR.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef WEBASSEMBLYSSA_INTERFACES
+#define WEBASSEMBLYSSA_INTERFACES
+
+include "mlir/IR/OpBase.td"
+include "mlir/IR/BuiltinAttributes.td"
+
+def WasmSSALabelLevelInterface : OpInterface<"WasmSSALabelLevelInterface"> {
+  let description = [{
+    Operation that defines one level of nesting for wasm branching.
+    These operation region can be targeted by branch instructions.
+  }];
+  let methods = [
+    InterfaceMethod<
+      /*desc=*/        "Returns the target block address",
+      /*returnType=*/  "::mlir::Block*",
+      /*methodName=*/  "getLabelTarget",
+      /*args=*/        (ins)
+    >
+  ];
+}
+
+def WasmSSALabelBranchingInterface : OpInterface<"WasmSSALabelBranchingInterface"> {
+  let description = [{
+    Wasm operation that targets a label for a jump.
+  }];
+  let methods = [
+    InterfaceMethod<
+      /*desc=*/        "Returns the number of context to break from",
+      /*returnType=*/  "size_t",
+      /*methodName=*/  "getExitLevel",
+      /*args=*/        (ins)
+    >,
+    InterfaceMethod<
+      /*desc=*/        "Returns the destination of this operation",
+      /*returnType=*/  "WasmSSALabelLevelInterface",
+      /*methodName=*/  "getTargetOp",
+      /*args=*/        (ins),
+      /*methodBody=*/ [{
+        return *WasmSSALabelBranchingInterface::getTargetOpFromBlock($_op.getOperation()->getBlock(), $_op.getExitLevel());
+      }]
+    >,
+    InterfaceMethod<
+      /*desc=*/        "Return the target control flow ops that defined the label of this operation",
+      /*returnType=*/  "::mlir::Block*",
+      /*methodName=*/  "getTarget",
+      /*args=*/        (ins),
+      /*methodBody=*/  [{}],
+      /*defaultImpl=*/ [{
+        auto op = mlir::cast<WasmSSALabelBranchingInterface>(this->getOperation());
+        return op.getTargetOp().getLabelTarget();
+      }]
+    >
+  ];
+  let extraClassDeclaration = [{
+    static ::llvm::FailureOr<WasmSSALabelLevelInterface> getTargetOpFromBlock(::mlir::Block *block, uint32_t level);
+  }];
+  let verify = [{return verifyWasmSSALabelBranchingInterface($_op);}];
+}
+
+def WasmSSAImportOpInterface : OpInterface<"WasmSSAImportOpInterface"> {
+  let description = [{
+    Operation that imports a symbol from an external wasm module;
+  }];
+
+  let methods = [
+    InterfaceMethod<
+      /*desc=*/        "Returns the module name for the import",
+      /*returnType=*/  "::llvm::StringRef",
+      /*methodName=*/  "getModuleName",
+      /*args=*/        (ins)
+      >,
+    InterfaceMethod<
+      /*desc=*/        "Returns the import name for the import",
+      /*returnType=*/  "::llvm::StringRef",
+      /*methodName=*/  "getImportName",
+      /*args=*/        (ins)
+      >,
+    InterfaceMethod<
+      /*desc=*/        "Returns the wasm index based symbol of the op",
+      /*returnType=*/  "::mlir::StringAttr",
+      /*methodName=*/  "getSymbolName",
+      /*args=*/        (ins),
+      /*methodBody=*/  [{}],
+      /*defaultImpl=*/ [{
+        auto op = mlir::cast<ConcreteOp>(this->getOperation());
+        return op.getSymNameAttr();
+      }]
+      >,
+    InterfaceMethod<
+      /*desc=*/        "Returns the qualified name of the import",
+      /*returnType=*/  "std::string",
+      /*methodName=*/  "getQualifiedImportName",
+      /*args=*/        (ins),
+      /*methodBody=*/  [{
+        return ($_op.getModuleName() + llvm::Twine{"::"} + $_op.getImportName()).str();
+      }]
+      >,
+  ];
+}
+
+def WasmSSAConstantExpressionInitializerInterface :
+          OpInterface<"WasmSSAConstantExpressionInitializerInterface"> {
+  let description = [{
+    Operation that must be constant initialized. This
+    interface adds a verifier that checks that all ops
+    within the initializer region are "constant expressions"
+    as defined by the WASM standard.
+  }];
+
+  let verify = [{ return detail::verifyConstantExpressionInterface($_op); }];
+}
+
+def WasmSSAConstantExprCheckInterface :
+          OpInterface<"WasmSSAConstantExprCheckInterface"> {
+  let description = [{
+    Base interface for operations that can be used in a Wasm Constant Expression.
+    It shouldn't be used directly, use one of the derived instead.
+  }];
+
+  let methods = [
+    InterfaceMethod<
+      /*desc=*/        [{
+        Returns success if the current operation is valid in a constant expression context.
+      }],
+      /*returnType=*/  "::mlir::LogicalResult",
+      /*methodName=*/  "isValidInConstantExpr",
+      /*args=*/        (ins),
+      /*methodBody=*/ [{
+        return $_op.verifyConstantExprValidity();
+      }]
+      >
+  ];
+}
+
+def WasmSSAContextuallyConstantExprInterface :
+          OpInterface<"WasmSSAContextuallyConstantExprInterface", [WasmSSAConstantExprCheckInterface]> {
+  let description = [{
+    Base interface for operations that can be used in a Wasm Constant Expression
+    depending on the context.
+  }];
+
+  let methods = [
+    InterfaceMethod<
+      /*desc=*/        [{
+        Returns success if the current operation is valid in a constant expression context.
+      }],
+      /*returnType=*/  "::mlir::LogicalResult",
+      /*methodName=*/  "verifyConstantExprValidity",
+      /*args=*/        (ins)
+      >
+  ];
+}
+
+def WasmSSAConstantExprInterface :
+          OpInterface<"WasmSSAConstantExprInterface", [WasmSSAConstantExprCheckInterface]> {
+  let description = [{
+    Base interface for operations that can always be used in a Wasm Constant Expression.
+  }];
+
+  let methods = [
+    InterfaceMethod<
+      /*desc=*/        [{
+        Returns success if the current operation is valid in a constant expression context.
+      }],
+      /*returnType=*/  "::mlir::LogicalResult",
+      /*methodName=*/  "verifyConstantExprValidity",
+      /*args=*/        (ins),
+      /*methodBody=*/ [{}],
+      /*DefaultImplementation=*/ [{ return success(); }]
+      >
+  ];
+}
+
+#endif // WEBASSEMBLYSSA_INTERFACES
diff --git a/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAOps.td b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAOps.td
new file mode 100644
index 0000000000000..3726a78c5978b
--- /dev/null
+++ b/mlir/include/mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAOps.td
@@ -0,0 +1,679 @@
+//===- WebAssemblySSAOps.td - WebAssemblySSA op definitions -*- tablegen -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef WEBASSEMBLYSSA_OPS
+#define WEBASSEMBLYSSA_OPS
+
+
+include "mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSABase.td"
+include "mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSATypes.td"
+include "mlir/Dialect/WebAssemblySSA/IR/WebAssemblySSAInterfaces.td"
+
+include "mlir/Interfaces/FunctionInterfaces.td"
+include "mlir/Interfaces/InferTypeOpInterface.td"
+include "mlir/IR/BuiltinAttributeInterfaces.td"
+include "mlir/IR/SymbolInterfaces.td"
+
+class WasmSSA_Op<string mnemonic, list<Trait> traits = []> :
+    Op<WasmSSA_Dialect, mnemonic, traits>;
+
+class WasmSSA_BlockLikeOp<string mnemonic, string summaryStr> :
+  WasmSSA_Op<mnemonic, [Terminator, DeclareOpInterfaceMethods<WasmSSALabelLevelInterface>]> {
+  let summary = summaryStr;
+  let arguments = (ins Variadic<WasmSSA_ValType>: $inputs);
+  let regions = (region AnyRegion: $body);
+  let successors = (successor AnySuccessor: $target);
+  let extraClassDeclaration = [{
+    ::mlir::Block* createBlock() {
+      auto &block = getBody().emplaceBlock();
+      for (auto input : getInputs())
+        block.addArgument(input.getType(), input.getLoc());
+      return &block;
+    }
+  }];
+  let assemblyFormat = "(`(`$inputs^`)` `:` type($inputs))? attr-dict  `:` $body `>` $target";
+}
+
+def WasmSSA_BlockOp : WasmSSA_BlockLikeOp<"block", "Create a nesting level"> {}
+
+def WasmSSA_LoopOp : WasmSSA_BlockLikeOp<"loop", "Create a nesting level similar to Block Op, except that it has itself as a successor."> {}
+
+def WasmSSA_BlockReturnOp : WasmSSA_Op<"block_return", [Terminator,
+    DeclareOpInterfaceMethods<WasmSSALabelBranchingInterface>]> {
+  let summary = "Return from the current block";
+  let arguments = (ins Variadic<WasmSSA_ValType>: $inputs);
+  let extraClassDeclaration = [{
+    ::mlir::Block* getTarget();
+  }];
+  let assemblyFormat = "($inputs^ `:` type($inputs))? attr-dict";
+}
+
+def WasmSSA_BranchIfOp : WasmSSA_Op<"branch_if", [
+    Terminator,
+    DeclareOpInterfaceMethods<WasmSSALabelBranchingInterface>]> {
+  let summary = "Jump to target level if condition has non-zero value";
+  let arguments = (ins I32: $condition,
+                       UI32Attr: $exitLevel,
+                       Variadic<WasmSSA_ValType>: $inputs);
+  let successors = (successor AnySuccessor: $elseSuccessor);
+  let assemblyFormat = "$condition `to` `level` $exitLevel (`with` `args`  `(`$inputs^ `:` type($inputs)`)`)?  `else` $elseSuccessor  attr-dict";
+}
+
+def WasmSSA_ConstOp : WasmSSA_Op<"const", [
+    AllTypesMatch<["value", "result"]>, WasmSSAConstantExprInterface]> {
+  let summary = "Push a constant to the stack";
+  let arguments = (ins TypedAttrInterface: $value);
+  let results = (outs WasmSSA_NumericType: $result);
+  let assemblyFormat = "$value attr-dict";
+}
+
+def WasmSSA_FuncOp : WasmSSA_Op<"func", [
+    AffineScope, AutomaticAllocationScope,
+    DeclareOpInterfaceMethods<FunctionOpInterface>,
+    IsolatedFromAbove,
+    Symbol]> {
+  let description = [{
+    Represents a Wasm function definition.
+
+    In Wasm function, locals and function arguments are interchangeable.
+    They are for instance both accessed using `local.get` instruction.
+
+    On the other hand, a function type is defined as a pair of tuples of Wasm value types.
+    To model this, the wasm.func operation has:
+
+    - A function type that represents the corresponding wasm type (tuples of value types)
+
+    - Arguments of the entry block of type `!wasm<local T>`, with T the corresponding type
+     in the function type.
+  }];
+  let arguments = (ins SymbolNameAttr: $sym_name,
+                     WasmSSA_FuncTypeAttr: $functionType,
+                     OptionalAttr<DictArrayAttr>:$arg_attrs,
+                     OptionalAttr<DictArrayAttr>:$res_attrs,
+                     DefaultValuedAttr<StrAttr, "\"nested\"">:$sym_visibility);
+  let regions = (region AnyRegion: $body);
+  let extraClassDeclaration = [{
+
+    /// Create the entry block for the function with parameters wrapped in local ref.
+    ::mlir::Block* addEntryBlock();
+
+    //===------------------------------------------------------------------===//
+    // FunctionOpInterface Methods
+    //===------------------------------------------------------------------===//
+
+    /// Returns the region on the current operation that is callable. This may
+    /// return null in the case of an external callable object, e.g. an external
+    /// function.
+    ::mlir::Region *getCallableRegion() { return isExternal() ? nullptr : &getBody(); }
+
+    ::mlir::LogicalResult verifyBody();
+
+    /// Returns the argument types of this function.
+    ArrayRef<Type> getArgumentTypes() { return getFunctionType().getInputs(); }
+
+    /// Returns the result types of this function.
+    ArrayRef<Type> getResultTypes() { return getFunctionType().getResults(); }
+  }];
+
+  let builders = [
+    OpBuilder<(ins "llvm::StringRef":$symbol, "FunctionType":$funcType )>
+  ];
+  let hasCustomAssemblyFormat = 1;
+}
+
+def WasmSSA_FuncCallOp : WasmSSA_Op<"call"> {
+  let summary = "Calling a wasm function";
+  let arguments = (ins FlatSymbolRefAttr: $callee,
+                       Variadic<WasmSSA_ValType>:  $operands);
+  let results = (outs Variadic<WasmSSA_ValType>: $results);
+  let assemblyFormat = "$callee (`(`$operands^`)`)? attr-dict `:` functional-type($operands, $results)";
+  let description = [{
+    Emits a call to callee. Takes a stack as input, which is used as the set of function arguments for callee.
+  }];
+}
+
+def WasmSSA_FuncImportOp : WasmSSA_Op<"import_func", [
+    Symbol,
+    CallableOpInterface,
+    WasmSSAImportOpInterface]> {
+  let summar...
[truncated]

Copy link

github-actions bot commented Jul 17, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- mlir/include/mlir/Dialect/WasmSSA/IR/WasmSSA.h mlir/include/mlir/Dialect/WasmSSA/IR/WasmSSAInterfaces.h mlir/lib/Dialect/WasmSSA/IR/WasmSSADialect.cpp mlir/lib/Dialect/WasmSSA/IR/WasmSSAInterfaces.cpp mlir/lib/Dialect/WasmSSA/IR/WasmSSAOps.cpp mlir/lib/Dialect/WasmSSA/IR/WasmSSATypes.cpp mlir/include/mlir/InitAllDialects.h
View the diff from clang-format here.
diff --git a/mlir/include/mlir/Dialect/WasmSSA/IR/WasmSSAInterfaces.h b/mlir/include/mlir/Dialect/WasmSSA/IR/WasmSSAInterfaces.h
index 518229ff6..236bc48e8 100644
--- a/mlir/include/mlir/Dialect/WasmSSA/IR/WasmSSAInterfaces.h
+++ b/mlir/include/mlir/Dialect/WasmSSA/IR/WasmSSAInterfaces.h
@@ -40,18 +40,21 @@ LogicalResult verifyLabelLevelInterface(Operation *op);
 } // namespace detail
 
 /// Operations implementing this trait are considered as valid
-/// constant expressions in any context (In contrast of ConstantExprCheckOpInterface
-/// which are sometimes considered valid constant expressions.
+/// constant expressions in any context (In contrast of
+/// ConstantExprCheckOpInterface which are sometimes considered valid constant
+/// expressions.
 template <class OperationType>
 struct AlwaysValidConstantExprOpTrait
-    : public OpTrait::TraitBase<OperationType, AlwaysValidConstantExprOpTrait> {};
+    : public OpTrait::TraitBase<OperationType, AlwaysValidConstantExprOpTrait> {
+};
 
 /// Trait used to verify operations that need a constant expression initializer.
-template<typename OpType>
-struct ConstantExpressionInitializerOpTrait : public OpTrait::TraitBase<OpType, ConstantExpressionInitializerOpTrait>{
-    static LogicalResult verifyTrait(Operation* op) {
-        return detail::verifyConstantExpressionInterface(op);
-    }
+template <typename OpType>
+struct ConstantExpressionInitializerOpTrait
+    : public OpTrait::TraitBase<OpType, ConstantExpressionInitializerOpTrait> {
+  static LogicalResult verifyTrait(Operation *op) {
+    return detail::verifyConstantExpressionInterface(op);
+  }
 };
 
 } // namespace mlir::wasmssa
diff --git a/mlir/lib/Dialect/WasmSSA/IR/WasmSSAInterfaces.cpp b/mlir/lib/Dialect/WasmSSA/IR/WasmSSAInterfaces.cpp
index 88fd5fb54..0a7a057a9 100644
--- a/mlir/lib/Dialect/WasmSSA/IR/WasmSSAInterfaces.cpp
+++ b/mlir/lib/Dialect/WasmSSA/IR/WasmSSAInterfaces.cpp
@@ -48,8 +48,8 @@ LogicalResult verifyConstantExpressionInterface(Operation *op) {
 }
 
 LogicalResult verifyLabelLevelInterface(Operation *op) {
-  Block* target = cast<LabelLevelOpInterface>(op).getLabelTarget();
-  Region* targetRegion = target->getParent();
+  Block *target = cast<LabelLevelOpInterface>(op).getLabelTarget();
+  Region *targetRegion = target->getParent();
   if (targetRegion != op->getParentRegion() ||
       targetRegion->getParentOp() != op)
     return op->emitError("target should be a block defined in same level than "
@@ -60,7 +60,7 @@ LogicalResult verifyLabelLevelInterface(Operation *op) {
 
 llvm::FailureOr<LabelLevelOpInterface>
 LabelBranchingOpInterface::getTargetOpFromBlock(::mlir::Block *block,
-                                                     uint32_t breakLevel) {
+                                                uint32_t breakLevel) {
   LabelLevelOpInterface res{};
   for (size_t curLevel{0}; curLevel <= breakLevel; curLevel++) {
     res = dyn_cast_or_null<LabelLevelOpInterface>(block->getParentOp());
diff --git a/mlir/lib/Dialect/WasmSSA/IR/WasmSSAOps.cpp b/mlir/lib/Dialect/WasmSSA/IR/WasmSSAOps.cpp
index fd5a4e9ae..e13002777 100644
--- a/mlir/lib/Dialect/WasmSSA/IR/WasmSSAOps.cpp
+++ b/mlir/lib/Dialect/WasmSSA/IR/WasmSSAOps.cpp
@@ -6,8 +6,8 @@
 //
 //===---------------------------------------------------------------------===//
 
-#include "mlir/Dialect/WasmSSA/IR/WasmSSAInterfaces.h"
 #include "mlir/Dialect/WasmSSA/IR/WasmSSA.h"
+#include "mlir/Dialect/WasmSSA/IR/WasmSSAInterfaces.h"
 
 #include "mlir/IR/Attributes.h"
 #include "mlir/IR/Builders.h"
@@ -266,9 +266,9 @@ LogicalResult GlobalGetOp::CheckValidInConstantExpr() {
   if (!symTableOp)
     return emitError(
         "cannot find the symbol table associated with this operation");
-  // NOTE: Having to lookup the symbol inside the symbol table anytime the verifier
-  // is called can be costly. This may be improved with caching or another architecture
-  // for the constant checking mechanism.
+  // NOTE: Having to lookup the symbol inside the symbol table anytime the
+  // verifier is called can be costly. This may be improved with caching or
+  // another architecture for the constant checking mechanism.
   Operation *definitionOp =
       SymbolTable::lookupSymbolIn(symTableOp, referencedSymbol);
   if (!definitionOp)
@@ -289,7 +289,8 @@ void GlobalImportOp::build(::mlir::OpBuilder &odsBuilder,
                            ::mlir::OperationState &odsState, StringRef symbol,
                            StringRef moduleName, StringRef importName,
                            Type type, bool isMutable) {
-  GlobalImportOp::build(odsBuilder, odsState, symbol, moduleName, importName, type, isMutable, odsBuilder.getStringAttr("nested"));
+  GlobalImportOp::build(odsBuilder, odsState, symbol, moduleName, importName,
+                        type, isMutable, odsBuilder.getStringAttr("nested"));
 }
 
 ParseResult GlobalImportOp::parse(OpAsmParser &parser, OperationState &result) {
@@ -384,9 +385,8 @@ LogicalResult LocalTeeOp::inferReturnTypes(
 }
 
 LogicalResult LocalTeeOp::verify() {
- if (getLocalVar().getType().getElementType() !=
-                     getValue().getType() ||
-                 getValue().getType() != getResult().getType())
+  if (getLocalVar().getType().getElementType() != getValue().getType() ||
+      getValue().getType() != getResult().getType())
     return emitError("input type and output type of local.tee do not match");
   return llvm::success();
 }
@@ -404,7 +404,8 @@ Block *LoopOp::getLabelTarget() { return &getBody().front(); }
 void MemOp::build(::mlir::OpBuilder &odsBuilder,
                   ::mlir::OperationState &odsState, llvm::StringRef symbol,
                   LimitType limit) {
-  MemOp::build(odsBuilder, odsState, symbol, limit, odsBuilder.getStringAttr("nested"));
+  MemOp::build(odsBuilder, odsState, symbol, limit,
+               odsBuilder.getStringAttr("nested"));
 }
 
 //===----------------------------------------------------------------------===//
@@ -448,7 +449,8 @@ void ReturnOp::build(::mlir::OpBuilder &odsBuilder,
 void TableOp::build(::mlir::OpBuilder &odsBuilder,
                     ::mlir::OperationState &odsState, llvm::StringRef symbol,
                     TableType type) {
-  TableOp::build(odsBuilder, odsState, symbol, type, odsBuilder.getStringAttr("nested"));
+  TableOp::build(odsBuilder, odsState, symbol, type,
+                 odsBuilder.getStringAttr("nested"));
 }
 
 //===----------------------------------------------------------------------===//

@flemairen6 flemairen6 force-pushed the ferdinand.start_wasm_mlir_upstream branch 2 times, most recently from 280772e to 1f3896e Compare July 17, 2025 02:38
Copy link
Member

@rengolin rengolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks more or less like I expected, which is mostly boilerplate for the new dialect. My only concern is that this PR adds a lot of operations but only tests a minor section of them. Not sure if more tests will be added later, but it's not clear from the description if that's the case, or a priority.

Given that there's no syntax helper on the ops description, a test file per op would not only be good engineering practice but also give us an idea how the dialect is represented.

def WasmSSALabelLevelInterface : OpInterface<"WasmSSALabelLevelInterface"> {
let description = [{
Operation that defines one level of nesting for wasm branching.
These operation region can be targeted by branch instructions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you guarantee the block is in the same region as the caller of the function?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that by caller of the function you mean same region as the op, as this interface is intended for Wasm structured control flow operations (block, if(else) and. loop).

I've added a verifier for this (checking that either the op and target share the same parent region, or that the target is inside the op region).

For the existing op implementing this interface this is done "automatically" though: if and block returns their successor which is already verified, and loop returns its region entry block.

/*returnType=*/ "size_t",
/*methodName=*/ "getExitLevel",
/*args=*/ (ins)
>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit scary to me from a design point of view: can you show an example of such an op and how it works?

Right now MLIR LangRef is restricted in terms of control-flow, and only terminators can end a block and they may only return to the immediate parent.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, so for instance (taken from the tests on our fork) - the last exemple of https://github.com/lforg37/llvm-project/blob/wasm.renamed_upstream_base/mlir/test/Target/Wasm/if.mlir

We use the block_return terminator op at end of any nesting. Later on when we lower it to other dialects, we "flatten" the nestings with branching to their respective original targets, there are some examples in https://github.com/lforg37/llvm-project/blob/wasm.renamed_upstream_base/mlir/test/Conversion/RaiseWasm/wasm-blocks-to-cf.mlir

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, the wasmssa.if is itself a terminator operation that has successor blocks. So the control flows from block_return to the parent wasmssa.if and then to its successor, forwarding the data along. This sounds fine wrt LangRef. Though we may have to double-check that control flow interfaces handle this correctly, i.e., one op can implement both RegionBranchOpInterface and RegionBranchTerminatorOpInterface without breaking anything.

What I don't see in the example is the nested-if situation. Is there a possibility to exit from multiple nesting levels immediately?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document this in the interface, in particular the requirement that the op must be a terminator itself (actually I think this should be in the verifier for the interface).

Copy link

@lforg37 lforg37 Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is indeed possible to exit multiple level at once.
Not with the block_return, but with the branching operations which can skip a level.
These operations have as attribute the number of nesting level to "escape".
The rationale was to mimic the Wasm way of doing, where some control flow integrity is ensured using restricted branching from inner nesting level to containing nesting level (conceptually equivalent to a break / continue statements that would accept a number of nesting level to skip) but not arbitrarily (not as permissive as a goto).

IIRC we tried to do it with successors in the branch ops, but the issue was that when skipping a level there is a jump to a block that is outside of current operation region. We will check how to fix this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way to fix the issue would be to add at each nest level a successor block for each possible / targeted parent nest.

E.g we would have something like this

block0 : block -> i32 {
    block1 : block -> i64, i64 {
       block2 : block -> f32, f32, f32 {
           // can contains branch to target any of the containing exit blocks
        } > ^block2_next, ^block_1_pass_through, block_0_pass_through
    ^block_2_next (f32, f32, f32):
       // this is the "natural" successor of block2, where its produced values are expected to be used
       // terminator gives control back to the block_1 op
  
    ^block_1_pass_through(i64, i64):
      // This is is a block that "forwards" the argument to block1_next: 
      // Control is returned to block1 operation, which transfers it to block1_next.
  
    ^block_0_pass_through(i32):
      // This ends the operation and gives back control to block1 op which will transfer it to the parent pass through block    
   }> block1_next, block_0_pass_through:
  ^block1_next(i64, i64):
     // this is the "natural" successor of block1, where its produced values are expected to be used
     // terminator gives control back to the block_1 op
  
  ^block_0_pass_through(i32):
    //This is is a block that "forwards" the argument to block1_next: 
    // Control is returned to block0 operation, which transfers it to block0_next
} >block0_next

^block0_next(i32):
     // this is the "natural" successor of block0, where its produced values are expected to be used

Which would create at most n new "passthrough" blocks for each block like of depth n.

Verifying the op correctness is still possible, but it makes it more complex to keep the mapping with Wasm branch representation.

Another solution is for us to wait for the implementation of the [Region-based control-flow with early exits in MLIR RFC](https://discourse.llvm.org/t/rfc-region-based-control-flow-with-early-exits-in-mlir/76998 to add the blocks back. They would be removed in the meantime, but that remove quite a lot of the expressiveness of the dialect.

Depending on the likeliness for it to land soon (maybe @joker-eph have information on this?) it might be the simplest path to have something both clean and respecting the langref.

Would keeping the current design while reinforcing the verifiers be a valid option?
The multi level jumps is in infraction with the LangRef, but I think it does it in a "conservative" way, in a sense that it can prevent optimizations but wouldn't create blocks that falsely appear to have no predecessor and be removed.

We do have on subsequent commits a conversion pass that lowers those construct by inlining all the block-like ops region in the function body and create the correct connections via cf operations. It is actually combined with all the other lowering, but we can make it standalone and count on it to "renormalize" the representation for the sake of control-flow based transformations?

Do you have other suggestions @ftynse?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g we would have something like this

I can't parse the example right now, is this supposed to be MLIR or some sort of pseudo-code?

Depending on the likeliness for it to land soon (maybe @joker-eph have information on this?)

I'm shooting for the end of August right now.

The multi level jumps is in infraction with the LangRef,

I do have a strong objection to anything that violates LangRef right now, but I didn't parse the example (didn't spent too much time either right now) so I would have to understand this more before emitting a definitive opinion on this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't parse the example right now, is this supposed to be MLIR or some sort of pseudo-code?

This is some MLIR flavored pseudo code. Let me try to reformulate it better in English, along with comments on why I think it do solve the issue (as my understanding of LangRef might get wrong).

First, current design issue is that there are operations that have a semantics of escaping multiple levels of region nesting at once. E.g. branch 2 in Wasm means roughly resume the execution at the end of the scope enclosing the scope enclosing the current scope. The current state of the PR is to mimic this, with some actual control flow path which are not reflected in the successor lists.

My understanding is that to make it compliant with LangRef we need to make the framework aware of all possible control flow paths via the successor mechanism. With successor of an op being in the same region that the op.

Currently a control flow op has only one successor which is the "continuation" block, where the control flow should go when the region of the op is executed completely without any early exit.

The proposal is to add one successor block for each of possible exit path.

Basically, at nesting level 0 there's only one exit level, early exit can only make you access it earlier.

At level 1 you can either early exit to level 1 continuation block or conceptually to level 0 continuation block, but this last one is not defined in an accessible region.
So a skip block in the enclosing region is added to have a path to exit that the framework is aware of.

In a more MLIR-ish way this would looks like

wasmssa.block() {
  WasmSSA.block() {
     // This would transfer the control flow back to the inner block op
     // It will then flow to ^level0skip
     wasmssa.branch 1 
     
     // This will continue to level1continuation 
     wasmssa.block_return 
     // (This is equivalent to wasmssa.branch 0)
  } ^level0skip, level1continuation
^level1continuation:
   // here are the instructions that follows inner block in top level block
^level0skip:
  wasmssa.branch 0
} > ^levelOcontinuation

^levelOcontinuation:
//

Inner block has two possible successors: either the continuation block to continue after it inside outer block, or the skip block that will forward the control flow to outer block continuation block.

Generally speaking a control flow op at nesting level n would have n+1 successors. Not all of them will be actual accessible path, we could have a cleaning step to remove all skip blocks that are not used by inner operation branching.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, current design issue is that there are operations that have a semantics of escaping multiple levels of region nesting at once. E.g. branch 2 in Wasm means roughly resume the execution at the end of the scope enclosing the scope enclosing the current scope.

This seems OK to me as long at the parent is a terminator as well. I would be more concerned with resuming at an arbitrary block in the parent scope.

The current state of the PR is to mimic this, with some actual control flow path which are not reflected in the successor lists.

It's OK, I see it as returning to the parent, with a flag instructing the parent to also return.

My understanding is that to make it compliant with LangRef we need to make the framework aware of all possible control flow paths via the successor mechanism.

My main concern with LangRef is that we shouldn't have an operation in the middle of a block that is returning. That means:

func.func @foo() {
   if (..) {
      return;
   }
   ...

This is not possible because from the function body block point of view, it looks like the if is interrupting the flow of execution directly while not being a terminator.

What you have looks just fine with me.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, then I'll just add a bit of doc and add verification that ops implementing the interface are terminators.
Sorry for the noise!

@flemairen6
Copy link
Author

This looks more or less like I expected, which is mostly boilerplate for the new dialect. My only concern is that this PR adds a lot of operations but only tests a minor section of them. Not sure if more tests will be added later, but it's not clear from the description if that's the case, or a priority.

Given that there's no syntax helper on the ops description, a test file per op would not only be good engineering practice but also give us an idea how the dialect is represented.

We wanted to avoid having too big of a PR at first to make it easier to review, so most of the tests would be upstreamed with the importer, the tool that translate binary wasm to wasm MLIR. The only things tested here are the operations with a custom format.
I can confirm we have a test file per operation. If you think it would be better to have an overview of the dialect, I can also push the importer and the tests to this PR but that will add a lot of work for reviewers. (It can be reviewed commit by commit though, we made sure to rewrote the history so every commit makes sense and they can be checked in a vacuum)

This dialect is an SSA representation of a WebAssembly program.

---------

Co-authored-by: Ferdinand Lemaire <ferdinand.lemaire@woven-planet.global>
Co-authored-by: Jessica Paquette <jessica.paquette@woven-planet.global>
@flemairen6 flemairen6 force-pushed the ferdinand.start_wasm_mlir_upstream branch from 1f3896e to d8b21ad Compare July 18, 2025 07:03
Copy link
Member

@ftynse ftynse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. My main concern/misunderstanding is with block exits

@flemairen6 flemairen6 force-pushed the ferdinand.start_wasm_mlir_upstream branch from 8392ee9 to f79a7d5 Compare July 22, 2025 04:41
Operation *symTableOp =
getOperation()->getParentWithTrait<OpTrait::SymbolTable>();
Operation *definitionOp =
SymbolTable::lookupSymbolIn(symTableOp, referencedSymbol);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's annoyingly gonna be costly to check, but I don't have a suggestion to improve this right now: ideally we should try to cache symbol tables within the verifier.

Maybe instead of doing this check in isValidInConstantExpr we could do it inside verifySymbolUses() by checking if a parent of GlobalGetOp implements ConstantExpressionInitializerOpTrait?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it might be costly on bigger projects. One solution might be to store "validated" symbols but I'm not sure it would bring great benefit, since this would get lost between two IR transformations.
I also don't think moving it to verifySymbolUses would change the situation a lot though since it would still run the same number of times, just from a different place so I'd like to refrain from doing that right now, I think we can improve this later.
I'll add a comment to acknowledge the issue.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also don't think moving it to verifySymbolUses would change the situation a lot though since it would still run the same number of times,

I think you missed an important motivation behind verifySymbolUses: it will build a symbol table once for the module, cache it and pass it around so that the lookup you're doing here becomes O(1).

if (maxLim)
printer << *maxLim;
printer << ']';
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as elsewhere: why can't this be a declarative assembly?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The datatype used to represent the boundaries of the limit type is uint32 - I tried to represent this in tablegen but had troubles representing an Optional for the max boundary. (Something like Missing cppType field in Attribute/Type parameter: anonymous_398) I tried to look for a dialect trying to represent something similar but couldn't find any.
I'm still having trouble with some parts of the ODS so if there is a way to represent this, I'd gladly move this in the tablegen file.

@flemairen6 flemairen6 force-pushed the ferdinand.start_wasm_mlir_upstream branch from 97b4bd9 to d0aed4d Compare July 25, 2025 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants