[WebAssembly] Add RefTypeMem2Local pass #81965
Conversation
This adds `WebAssemblyRefTypeMem2Local` pass, which changes the address spaces of reference type `alloca`s to `addrspace(1)`. This in turn changes the address spaces of all `load` and `store` instructions that use the `alloca`s. `addrspace(1)` is `WASM_ADDRESS_SPACE_VAR`, and loads and stores to this address space become `local.get`s and `local.set`s, thanks to the Wasm local IR support added in llvm@82f92e3. In a follow-up PR, I am planning to replace the usage of mem2reg pass with this to solve the reference type `alloca` problems described in llvm#81575.
|
@llvm/pr-subscribers-backend-webassembly Author: Heejin Ahn (aheejin) ChangesThis adds
In a follow-up PR, I am planning to replace the usage of mem2reg pass with this to solve the reference type Full diff: https://github.com/llvm/llvm-project/pull/81965.diff 5 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/CMakeLists.txt b/llvm/lib/Target/WebAssembly/CMakeLists.txt
index bb2ccea5c14598..f430be2653b4ee 100644
--- a/llvm/lib/Target/WebAssembly/CMakeLists.txt
+++ b/llvm/lib/Target/WebAssembly/CMakeLists.txt
@@ -43,6 +43,7 @@ add_llvm_target(WebAssemblyCodeGen
WebAssemblyOptimizeLiveIntervals.cpp
WebAssemblyOptimizeReturned.cpp
WebAssemblyPeephole.cpp
+ WebAssemblyRefTypeMem2Local.cpp
WebAssemblyRegisterInfo.cpp
WebAssemblyRegColoring.cpp
WebAssemblyRegNumbering.cpp
diff --git a/llvm/lib/Target/WebAssembly/WebAssembly.h b/llvm/lib/Target/WebAssembly/WebAssembly.h
index 91765ad117bdb0..1c40addb6d6f78 100644
--- a/llvm/lib/Target/WebAssembly/WebAssembly.h
+++ b/llvm/lib/Target/WebAssembly/WebAssembly.h
@@ -30,6 +30,7 @@ ModulePass *createWebAssemblyAddMissingPrototypes();
ModulePass *createWebAssemblyFixFunctionBitcasts();
FunctionPass *createWebAssemblyOptimizeReturned();
FunctionPass *createWebAssemblyLowerRefTypesIntPtrConv();
+FunctionPass *createWebAssemblyRefTypeMem2Local();
// ISel and immediate followup passes.
FunctionPass *createWebAssemblyISelDag(WebAssemblyTargetMachine &TM,
@@ -59,6 +60,7 @@ ModulePass *createWebAssemblyMCLowerPrePass();
// PassRegistry initialization declarations.
void initializeFixFunctionBitcastsPass(PassRegistry &);
void initializeOptimizeReturnedPass(PassRegistry &);
+void initializeWebAssemblyRefTypeMem2LocalPass(PassRegistry &);
void initializeWebAssemblyAddMissingPrototypesPass(PassRegistry &);
void initializeWebAssemblyArgumentMovePass(PassRegistry &);
void initializeWebAssemblyCFGSortPass(PassRegistry &);
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyRefTypeMem2Local.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyRefTypeMem2Local.cpp
new file mode 100644
index 00000000000000..45773fd179184d
--- /dev/null
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyRefTypeMem2Local.cpp
@@ -0,0 +1,91 @@
+//=== WebAssemblyRefTypeMem2Local.cpp - WebAssembly RefType Mem2Local -----===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Assign reference type allocas to local addrspace (addrspace(1)) so that
+/// their loads and stores can be lowered to local.gets/local.sets.
+///
+//===----------------------------------------------------------------------===//
+
+#include "Utils/WasmAddressSpaces.h"
+#include "Utils/WebAssemblyTypeUtilities.h"
+#include "WebAssembly.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/InstVisitor.h"
+#include "llvm/IR/ValueHandle.h"
+#include "llvm/Pass.h"
+using namespace llvm;
+
+#define DEBUG_TYPE "wasm-ref-type-mem2local"
+
+namespace {
+class WebAssemblyRefTypeMem2Local final
+ : public FunctionPass,
+ public InstVisitor<WebAssemblyRefTypeMem2Local> {
+ StringRef getPassName() const override {
+ return "WebAssembly Refernce Types Memory to Local";
+ }
+
+ void getAnalysisUsage(AnalysisUsage &AU) const override {
+ AU.setPreservesCFG();
+ FunctionPass::getAnalysisUsage(AU);
+ }
+
+ bool runOnFunction(Function &F) override;
+ bool Changed = false;
+
+public:
+ static char ID;
+ WebAssemblyRefTypeMem2Local() : FunctionPass(ID) {}
+
+ void visitAllocaInst(AllocaInst &AI);
+};
+} // End anonymous namespace
+
+char WebAssemblyRefTypeMem2Local::ID = 0;
+INITIALIZE_PASS(WebAssemblyRefTypeMem2Local, DEBUG_TYPE,
+ "Assign reference type allocas to local address space", true,
+ false)
+
+FunctionPass *llvm::createWebAssemblyRefTypeMem2Local() {
+ return new WebAssemblyRefTypeMem2Local();
+}
+
+void WebAssemblyRefTypeMem2Local::visitAllocaInst(AllocaInst &AI) {
+ if (WebAssembly::isWebAssemblyReferenceType(AI.getAllocatedType())) {
+ Changed = true;
+ IRBuilder<> IRB(AI.getContext());
+ IRB.SetInsertPoint(&AI);
+ auto *NewAI = IRB.CreateAlloca(AI.getAllocatedType(),
+ WebAssembly::WASM_ADDRESS_SPACE_VAR, nullptr,
+ AI.getName() + ".var");
+
+ // The below is basically equivalent to AI.replaceAllUsesWith(NewAI), but we
+ // cannot use it because it requires the old and new types be the same,
+ // which is not true here because the address spaces are different.
+ if (AI.hasValueHandle())
+ ValueHandleBase::ValueIsRAUWd(&AI, NewAI);
+ if (AI.isUsedByMetadata())
+ ValueAsMetadata::handleRAUW(&AI, NewAI);
+ while (!AI.materialized_use_empty()) {
+ Use &U = *AI.materialized_use_begin();
+ U.set(NewAI);
+ }
+
+ AI.eraseFromParent();
+ }
+}
+
+bool WebAssemblyRefTypeMem2Local::runOnFunction(Function &F) {
+ LLVM_DEBUG(dbgs() << "********** WebAssembly RefType Mem2Local **********\n"
+ "********** Function: "
+ << F.getName() << '\n');
+
+ visit(F);
+ return Changed;
+}
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
index 42043a7b8680a4..681e39e5aa5e32 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -68,6 +68,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeWebAssemblyTarget() {
initializeLowerGlobalDtorsLegacyPassPass(PR);
initializeFixFunctionBitcastsPass(PR);
initializeOptimizeReturnedPass(PR);
+ initializeWebAssemblyRefTypeMem2LocalPass(PR);
initializeWebAssemblyArgumentMovePass(PR);
initializeWebAssemblySetP2AlignOperandsPass(PR);
initializeWebAssemblyReplacePhysRegsPass(PR);
diff --git a/llvm/test/CodeGen/WebAssembly/ref-type-mem2local.ll b/llvm/test/CodeGen/WebAssembly/ref-type-mem2local.ll
new file mode 100644
index 00000000000000..8f09b25cca8d05
--- /dev/null
+++ b/llvm/test/CodeGen/WebAssembly/ref-type-mem2local.ll
@@ -0,0 +1,39 @@
+; RUN: opt < %s -wasm-ref-type-mem2local -S | FileCheck %s
+
+target triple = "wasm32-unknown-unknown"
+
+%externref = type ptr addrspace(10)
+%funcref = type ptr addrspace(20)
+
+declare %funcref @get_funcref()
+declare %externref @get_externref()
+declare void @take_funcref(%funcref)
+declare void @take_externref(%externref)
+
+; CHECK-LABEL: @test_ref_type_mem2local
+define void @test_ref_type_mem2local() {
+entry:
+ %alloc.externref = alloca %externref, align 1
+ %eref = call %externref @get_externref()
+ store %externref %eref, ptr %alloc.externref, align 1
+ %eref.loaded = load %externref, ptr %alloc.externref, align 1
+ call void @take_externref(%externref %eref.loaded)
+ ; CHECK: %alloc.externref.var = alloca ptr addrspace(10), align 1, addrspace(1)
+ ; CHECK-NEXT: %eref = call ptr addrspace(10) @get_externref()
+ ; CHECK-NEXT: store ptr addrspace(10) %eref, ptr addrspace(1) %alloc.externref.var, align 1
+ ; CHECK-NEXT: %eref.loaded = load ptr addrspace(10), ptr addrspace(1) %alloc.externref.var, align 1
+ ; CHECK-NEXT: call void @take_externref(ptr addrspace(10) %eref.loaded)
+
+ %alloc.funcref = alloca %funcref, align 1
+ %fref = call %funcref @get_funcref()
+ store %funcref %fref, ptr %alloc.funcref, align 1
+ %fref.loaded = load %funcref, ptr %alloc.funcref, align 1
+ call void @take_funcref(%funcref %fref.loaded)
+ ; CHECK-NEXT: %alloc.funcref.var = alloca ptr addrspace(20), align 1, addrspace(1)
+ ; CHECK-NEXT: %fref = call ptr addrspace(20) @get_funcref()
+ ; CHECK-NEXT: store ptr addrspace(20) %fref, ptr addrspace(1) %alloc.funcref.var, align 1
+ ; CHECK-NEXT: %fref.loaded = load ptr addrspace(20), ptr addrspace(1) %alloc.funcref.var, align 1
+ ; CHECK-NEXT: call void @take_funcref(ptr addrspace(20) %fref.loaded)
+
+ ret void
+}
|
|
@xortoast Thanks for the hint! I tried to add you as a reviewer but couldn't, because I guess you don't have a committer access. But I'd appreciate if you take a look. |
| // The below is basically equivalent to AI.replaceAllUsesWith(NewAI), but we | ||
| // cannot use it because it requires the old and new types be the same, | ||
| // which is not true here because the address spaces are different. |
There was a problem hiding this comment.
I couldn't do AI.replaceAllUsesWith(NewAI) because of this assertion:
llvm-project/llvm/lib/IR/Value.cpp
Lines 507 to 508 in 3af5c98
There was a problem hiding this comment.
Does the IR verifier still pass after this transformation, or does it also check this invariant?
There was a problem hiding this comment.
Running -verify after this pass doesn't report any error.
|
Would it be beneficial to expand this pass to all types (where possible), not just reference types? The main argument against unconditionally running In fact, this would make debugging easier (especially in situations where DWARF debug info is not available), as well as produce simpler and faster code in unoptimized builds. |
|
Sorry for the late reply! I was OOO for a while.
Correct me if I'm mistaken. But it looks the MMI table only works with memory, so currently the %alloc.externref = alloca ptr addrspace(10), align 1, !dbg !17
tail call void @llvm.dbg.declare(metadata ptr %alloc.externref, metadata !9, metadata !DIExpression()), !dbg !17
...
%alloc.funcref = alloca ptr addrspace(20), align 1, !dbg !22
tail call void @llvm.dbg.declare(metadata ptr %alloc.funcref, metadata !14, metadata !DIExpression()), !dbg !22
...
%alloc.i32 = alloca i32, align 4, !dbg !33
tail call void @llvm.dbg.declare(metadata ptr %alloc.i32, metadata !30, metadata !DIExpression()), !dbg !33and compile it to an object file and using So I'm not sure what you mean by that debug info is not an issue here, because it looks debug info is lost. I think what we should do is to generate appropriate Given that reference type variables are supposedly rare, I don't think this loss of debug info would affect the debug info coverage a lot yet, but doing the same thing for all types will likely delete all variable debug info at this point. Even if we add the
Why would it make debugging easier? I also don't understand what situation you mean by "where DWARF debug info is not available", because I've been assuming DWARF info when talking about debug info in LLVM. Are there other framework we use to cover variable debug info? Also I think it's OK for unoptimized builds to be slow. |
|
@aheejin have you tried removing the mem2reg pass and see if things still work? AFAIR, it was mem2reg which held this together and it was forcefully enabled at all optimization levels. |
|
@pmatos Yes, replacing the mem2reg with this pass seems to work on all existing LLVM and Emscripten reference type tests. I am planning to upload that, I mean the replacing, as a follow-up to this PR. |
Excellent. LGTM. It turned out simpler than what I had imagined. Nice patch. |
|
@xortoast Do you have remaining concerns? |
|
@aheejin Sorry for the late reply.
I was not aware of that.
I was referring to runtimes that don't support DWARF and cases where DWARF debug info is lost due to post-processing. In those cases you can only see raw locals and raw linear memory, and I think it's easier to work with the former.
You can attach names to local variables with the In any case, my previous comment was based on some incorrect assumptions. You can merge this PR now, and we can continue this discussion later when the situation with debug info changes. |
When reference-types feature is enabled, forcing mem2reg unconditionally even in `-O0` has some problems described in llvm#81575. This uses RefTypeMem2Local pass added in llvm#81965 instead. This also removes `IsForced` parameter added in llvm@890146b given that we don't need it anymore. This may still hurt debug info related to reference type variables a little during the backend transformation given that they are not stored in memory anymore, but reference type variables are presumably rare and it would be still a lot less damage than forcing mem2reg on the whole program. Also this fixes the EH problem described in llvm#81575. Fixes llvm#81575.
When reference-types feature is enabled, forcing mem2reg unconditionally even in `-O0` has some problems described in #81575. This uses RefTypeMem2Local pass added in #81965 instead. This also removes `IsForced` parameter added in 890146b given that we don't need it anymore. This may still hurt debug info related to reference type variables a little during the backend transformation given that they are not stored in memory anymore, but reference type variables are presumably rare and it would be still a lot less damage than forcing mem2reg on the whole program. Also this fixes the EH problem described in #81575. Fixes #81575.
This adds
WebAssemblyRefTypeMem2Localpass, which changes the address spaces of reference typeallocas toaddrspace(1). This in turn changes the address spaces of allloadandstoreinstructions that use theallocas.addrspace(1)isWASM_ADDRESS_SPACE_VAR, and loads and stores to this address space becomelocal.gets andlocal.sets, thanks to the Wasm local IR support added in82f92e3.
In a follow-up PR, I am planning to replace the usage of mem2reg pass with this to solve the reference type
allocaproblems described in #81575.