Skip to content

Commit

Permalink
Revert "[WebAssembly] Disable multivalue emission temporarily (#82714)"
Browse files Browse the repository at this point in the history
This reverts commit 6e6bf9f.

It turned out the multivalue feature had active outside users and it
could cause some disruptions to them, so I'd like to investigate more
about the workarounds before doing this.
  • Loading branch information
aheejin committed Feb 28, 2024
1 parent 062cfad commit 8506a63
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 43 deletions.
7 changes: 2 additions & 5 deletions llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ using namespace llvm;

#define DEBUG_TYPE "wasm-lower"

extern cl::opt<bool> WasmEmitMultiValue;

WebAssemblyTargetLowering::WebAssemblyTargetLowering(
const TargetMachine &TM, const WebAssemblySubtarget &STI)
: TargetLowering(TM), Subtarget(&STI) {
Expand Down Expand Up @@ -1290,16 +1288,15 @@ bool WebAssemblyTargetLowering::CanLowerReturn(
const SmallVectorImpl<ISD::OutputArg> &Outs,
LLVMContext & /*Context*/) const {
// WebAssembly can only handle returning tuples with multivalue enabled
return (Subtarget->hasMultivalue() && WasmEmitMultiValue) || Outs.size() <= 1;
return Subtarget->hasMultivalue() || Outs.size() <= 1;
}

SDValue WebAssemblyTargetLowering::LowerReturn(
SDValue Chain, CallingConv::ID CallConv, bool /*IsVarArg*/,
const SmallVectorImpl<ISD::OutputArg> &Outs,
const SmallVectorImpl<SDValue> &OutVals, const SDLoc &DL,
SelectionDAG &DAG) const {
assert(((Subtarget->hasMultivalue() && WasmEmitMultiValue) ||
Outs.size() <= 1) &&
assert((Subtarget->hasMultivalue() || Outs.size() <= 1) &&
"MVP WebAssembly can only return up to one value");
if (!callingConvSupported(CallConv))
fail(DL, DAG, "WebAssembly doesn't support non-C calling conventions");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
#include "llvm/Target/TargetMachine.h"
using namespace llvm;

extern cl::opt<bool> WasmEmitMultiValue;

WebAssemblyFunctionInfo::~WebAssemblyFunctionInfo() = default; // anchor.

MachineFunctionInfo *WebAssemblyFunctionInfo::clone(
Expand Down Expand Up @@ -73,8 +71,7 @@ void llvm::computeSignatureVTs(const FunctionType *Ty,

MVT PtrVT = MVT::getIntegerVT(TM.createDataLayout().getPointerSizeInBits());
if (Results.size() > 1 &&
(!TM.getSubtarget<WebAssemblySubtarget>(ContextFunc).hasMultivalue() ||
!WasmEmitMultiValue)) {
!TM.getSubtarget<WebAssemblySubtarget>(ContextFunc).hasMultivalue()) {
// WebAssembly can't lower returns of multiple values without demoting to
// sret unless multivalue is enabled (see
// WebAssemblyTargetLowering::CanLowerReturn). So replace multiple return
Expand Down
26 changes: 12 additions & 14 deletions llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@

using namespace llvm;

extern cl::opt<bool> WasmEmitMultiValue;

namespace {

enum RuntimeLibcallSignature {
Expand Down Expand Up @@ -696,7 +694,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(PtrTy);
break;
case i64_i64_func_f32:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
} else {
Expand All @@ -705,7 +703,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(wasm::ValType::F32);
break;
case i64_i64_func_f64:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
} else {
Expand All @@ -714,7 +712,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(wasm::ValType::F64);
break;
case i16_i16_func_i16_i16:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I32);
Rets.push_back(wasm::ValType::I32);
} else {
Expand All @@ -724,7 +722,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(wasm::ValType::I32);
break;
case i32_i32_func_i32_i32:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I32);
Rets.push_back(wasm::ValType::I32);
} else {
Expand All @@ -734,7 +732,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(wasm::ValType::I32);
break;
case i64_i64_func_i64_i64:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
} else {
Expand All @@ -744,7 +742,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(wasm::ValType::I64);
break;
case i64_i64_func_i64_i64_i64_i64:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
} else {
Expand All @@ -756,7 +754,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(wasm::ValType::I64);
break;
case i64_i64_func_i64_i64_i64_i64_iPTR:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
} else {
Expand All @@ -769,7 +767,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(PtrTy);
break;
case i64_i64_i64_i64_func_i64_i64_i64_i64:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
Expand All @@ -783,7 +781,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(wasm::ValType::I64);
break;
case i64_i64_func_i64_i64_i32:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
} else {
Expand Down Expand Up @@ -853,7 +851,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(wasm::ValType::I64);
break;
case i64_i64_func_i64_i64_i64_i64_i64_i64:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
} else {
Expand All @@ -867,7 +865,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(wasm::ValType::I64);
break;
case i64_i64_func_i32:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
} else {
Expand All @@ -876,7 +874,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget,
Params.push_back(wasm::ValType::I32);
break;
case i64_i64_func_i64:
if (Subtarget.hasMultivalue() && WasmEmitMultiValue) {
if (Subtarget.hasMultivalue()) {
Rets.push_back(wasm::ValType::I64);
Rets.push_back(wasm::ValType::I64);
} else {
Expand Down
9 changes: 0 additions & 9 deletions llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,6 @@ static cl::opt<bool> WasmDisableFixIrreducibleControlFlowPass(
" irreducible control flow optimization pass"),
cl::init(false));

// A temporary option to control emission of multivalue until multivalue
// implementation is stable enough. We currently don't emit multivalue by
// default even if the feature section allows it.
// TODO Stabilize multivalue and delete this option
cl::opt<bool>
WasmEmitMultiValue("wasm-emit-multivalue", cl::Hidden,
cl::desc("WebAssembly: Emit multivalue in the backend"),
cl::init(false));

extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeWebAssemblyTarget() {
// Register the target.
RegisterTargetMachine<WebAssemblyTargetMachine> X(
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj-multi-return.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: not --crash llc < %s -enable-emscripten-cxx-exceptions -mattr=+multivalue -wasm-emit-multivalue 2>&1 | FileCheck %s --check-prefix=EH
; RUN: not --crash llc < %s -enable-emscripten-sjlj -mattr=+multivalue 2>&1 -wasm-emit-multivalue | FileCheck %s --check-prefix=SJLJ
; RUN: not --crash llc < %s -enable-emscripten-cxx-exceptions -mattr=+multivalue 2>&1 | FileCheck %s --check-prefix=EH
; RUN: not --crash llc < %s -enable-emscripten-sjlj -mattr=+multivalue 2>&1 | FileCheck %s --check-prefix=SJLJ

; Currently multivalue returning functions are not supported in Emscripten EH /
; SjLj. Make sure they error out.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -mtriple=wasm32-unknown-unknown -mattr=+multivalue -wasm-emit-multivalue -run-pass=wasm-reg-stackify -verify-machineinstrs %s -o - | FileCheck %s
# RUN: llc -mtriple=wasm32-unknown-unknown -mattr=+multivalue -run-pass=wasm-reg-stackify -verify-machineinstrs %s -o - | FileCheck %s

--- |
target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20"
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/WebAssembly/multivalue-stackify.ll
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; NOTE: Test functions have been generated by multivalue-stackify.py.

; RUN: llc < %s -verify-machineinstrs -mattr=+multivalue -wasm-emit-multivalue | FileCheck %s
; RUN: llc < %s -verify-machineinstrs -mattr=+multivalue | FileCheck %s

; Test that the multivalue stackification works

Expand Down
10 changes: 4 additions & 6 deletions llvm/test/CodeGen/WebAssembly/multivalue.ll
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -mcpu=mvp -mattr=+multivalue,+tail-call -wasm-emit-multivalue | FileCheck %s
; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -mcpu=mvp -mattr=+reference-types,+multivalue,+tail-call -wasm-emit-multivalue | FileCheck --check-prefix REF %s
; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mcpu=mvp -mattr=+multivalue,+tail-call -wasm-emit-multivalue | FileCheck %s --check-prefix REGS
; RUN: llc < %s --filetype=obj -mcpu=mvp -mattr=+multivalue,+tail-call -wasm-emit-multivalue | obj2yaml | FileCheck %s --check-prefix OBJ
; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -mcpu=mvp -mattr=+multivalue,+tail-call | FileCheck %s --check-prefix NO-MULTIVALUE
; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -mcpu=mvp -mattr=+multivalue,+tail-call | FileCheck %s
; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -mcpu=mvp -mattr=+reference-types,+multivalue,+tail-call | FileCheck --check-prefix REF %s
; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mcpu=mvp -mattr=+multivalue,+tail-call | FileCheck %s --check-prefix REGS
; RUN: llc < %s --filetype=obj -mcpu=mvp -mattr=+multivalue,+tail-call | obj2yaml | FileCheck %s --check-prefix OBJ

; Test that the multivalue calls, returns, function types, and block
; types work as expected.
Expand All @@ -20,7 +19,6 @@ declare void @use_i64(i64)
; CHECK-NEXT: i32.const 42{{$}}
; CHECK-NEXT: i64.const 42{{$}}
; CHECK-NEXT: end_function{{$}}
; NO-MULTIVALUE-NOT: .functype pair_const () -> (i32, i64)
define %pair @pair_const() {
ret %pair { i32 42, i64 42 }
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/WebAssembly/multivalue_libcall.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
; RUN: llc < %s -verify-machineinstrs -mcpu=mvp -mattr=+multivalue -wasm-emit-multivalue | FileCheck %s --check-prefix=MULTIVALUE
; RUN: llc < %s -verify-machineinstrs -mcpu=mvp -mattr=+multivalue | FileCheck %s --check-prefix=MULTIVALUE
; RUN: llc < %s -verify-machineinstrs -mcpu=mvp | FileCheck %s --check-prefix=NO_MULTIVALUE

; Test libcall signatures when multivalue is enabled and disabled
Expand Down

0 comments on commit 8506a63

Please sign in to comment.