From be906f35f3aa6ddfb9d1c823aa23545a967bdcd8 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 9 Feb 2024 00:57:55 +0000 Subject: [PATCH 1/2] [WebAssembly] Disable multivalue emission temporarily We plan to enable multivalue in the features section soon (#80923) for other reasons, such as the feature having been standardized for many years and other features being developed (e.g. EH) depending on it. This is separate from enabling Clang experimental multivalue ABI (`-Xclang -target-abi -Xclang experimental-mv`), but it turned out we generate some multivalue code in the backend as well if it is enabled in the features section. Given that our backend multivalue generation still has not been much used nor tested, and enabling the feature in the features section can be a separate decision from how much multialue (including none) we decide to generate for now, I'd like to temporarily disable the actual generation of multivalue in our backend. To do that, this adds an internal flag `-wasm-emit-multivalue` that defaults to false. All our existing multivalue tests can use this to test multivalue code. This flag can be removed later when we are confident the multivalue generation is well tested. --- .../WebAssembly/WebAssemblyISelLowering.cpp | 7 +++-- .../WebAssemblyMachineFunctionInfo.cpp | 5 +++- .../WebAssemblyRuntimeLibcallSignatures.cpp | 26 ++++++++++--------- .../WebAssembly/WebAssemblyTargetMachine.cpp | 9 +++++++ .../lower-em-ehsjlj-multi-return.ll | 4 +-- .../multivalue-dont-move-def-past-use.mir | 2 +- .../WebAssembly/multivalue-stackify.ll | 2 +- llvm/test/CodeGen/WebAssembly/multivalue.ll | 10 ++++--- .../CodeGen/WebAssembly/multivalue_libcall.ll | 2 +- 9 files changed, 43 insertions(+), 24 deletions(-) diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp index 7c47790d1e351..36f067956e63a 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp @@ -43,6 +43,8 @@ using namespace llvm; #define DEBUG_TYPE "wasm-lower" +extern cl::opt WasmEmitMultiValue; + WebAssemblyTargetLowering::WebAssemblyTargetLowering( const TargetMachine &TM, const WebAssemblySubtarget &STI) : TargetLowering(TM), Subtarget(&STI) { @@ -1288,7 +1290,7 @@ bool WebAssemblyTargetLowering::CanLowerReturn( const SmallVectorImpl &Outs, LLVMContext & /*Context*/) const { // WebAssembly can only handle returning tuples with multivalue enabled - return Subtarget->hasMultivalue() || Outs.size() <= 1; + return (Subtarget->hasMultivalue() && WasmEmitMultiValue) || Outs.size() <= 1; } SDValue WebAssemblyTargetLowering::LowerReturn( @@ -1296,7 +1298,8 @@ SDValue WebAssemblyTargetLowering::LowerReturn( const SmallVectorImpl &Outs, const SmallVectorImpl &OutVals, const SDLoc &DL, SelectionDAG &DAG) const { - assert((Subtarget->hasMultivalue() || Outs.size() <= 1) && + assert(((Subtarget->hasMultivalue() && WasmEmitMultiValue) || + 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"); diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyMachineFunctionInfo.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyMachineFunctionInfo.cpp index 1e959111a4dbc..b969b8370a3e5 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyMachineFunctionInfo.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyMachineFunctionInfo.cpp @@ -22,6 +22,8 @@ #include "llvm/Target/TargetMachine.h" using namespace llvm; +extern cl::opt WasmEmitMultiValue; + WebAssemblyFunctionInfo::~WebAssemblyFunctionInfo() = default; // anchor. MachineFunctionInfo *WebAssemblyFunctionInfo::clone( @@ -71,7 +73,8 @@ void llvm::computeSignatureVTs(const FunctionType *Ty, MVT PtrVT = MVT::getIntegerVT(TM.createDataLayout().getPointerSizeInBits()); if (Results.size() > 1 && - !TM.getSubtarget(ContextFunc).hasMultivalue()) { + (!TM.getSubtarget(ContextFunc).hasMultivalue() || + !WasmEmitMultiValue)) { // WebAssembly can't lower returns of multiple values without demoting to // sret unless multivalue is enabled (see // WebAssemblyTargetLowering::CanLowerReturn). So replace multiple return diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp index 3e2e029695ab6..2a84c90c89602 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp @@ -24,6 +24,8 @@ using namespace llvm; +extern cl::opt WasmEmitMultiValue; + namespace { enum RuntimeLibcallSignature { @@ -694,7 +696,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget, Params.push_back(PtrTy); break; case i64_i64_func_f32: - if (Subtarget.hasMultivalue()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); } else { @@ -703,7 +705,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget, Params.push_back(wasm::ValType::F32); break; case i64_i64_func_f64: - if (Subtarget.hasMultivalue()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); } else { @@ -712,7 +714,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget, Params.push_back(wasm::ValType::F64); break; case i16_i16_func_i16_i16: - if (Subtarget.hasMultivalue()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I32); Rets.push_back(wasm::ValType::I32); } else { @@ -722,7 +724,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget, Params.push_back(wasm::ValType::I32); break; case i32_i32_func_i32_i32: - if (Subtarget.hasMultivalue()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I32); Rets.push_back(wasm::ValType::I32); } else { @@ -732,7 +734,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget, Params.push_back(wasm::ValType::I32); break; case i64_i64_func_i64_i64: - if (Subtarget.hasMultivalue()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); } else { @@ -742,7 +744,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()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); } else { @@ -754,7 +756,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()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); } else { @@ -767,7 +769,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()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); @@ -781,7 +783,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget, Params.push_back(wasm::ValType::I64); break; case i64_i64_func_i64_i64_i32: - if (Subtarget.hasMultivalue()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); } else { @@ -851,7 +853,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()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); } else { @@ -865,7 +867,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget, Params.push_back(wasm::ValType::I64); break; case i64_i64_func_i32: - if (Subtarget.hasMultivalue()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); } else { @@ -874,7 +876,7 @@ void llvm::getLibcallSignature(const WebAssemblySubtarget &Subtarget, Params.push_back(wasm::ValType::I32); break; case i64_i64_func_i64: - if (Subtarget.hasMultivalue()) { + if (Subtarget.hasMultivalue() && WasmEmitMultiValue) { Rets.push_back(wasm::ValType::I64); Rets.push_back(wasm::ValType::I64); } else { diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp index 42043a7b8680a..8814d0570dc41 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp @@ -54,6 +54,15 @@ static cl::opt 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 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 X( diff --git a/llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj-multi-return.ll b/llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj-multi-return.ll index 4f33439db770d..daf46c6eef025 100644 --- a/llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj-multi-return.ll +++ b/llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj-multi-return.ll @@ -1,5 +1,5 @@ -; 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 +; 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 ; Currently multivalue returning functions are not supported in Emscripten EH / ; SjLj. Make sure they error out. diff --git a/llvm/test/CodeGen/WebAssembly/multivalue-dont-move-def-past-use.mir b/llvm/test/CodeGen/WebAssembly/multivalue-dont-move-def-past-use.mir index 4b4661b144667..4fadbd5f07e6d 100644 --- a/llvm/test/CodeGen/WebAssembly/multivalue-dont-move-def-past-use.mir +++ b/llvm/test/CodeGen/WebAssembly/multivalue-dont-move-def-past-use.mir @@ -1,5 +1,5 @@ # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py -# RUN: llc -mtriple=wasm32-unknown-unknown -mattr=+multivalue -run-pass=wasm-reg-stackify -verify-machineinstrs %s -o - | FileCheck %s +# RUN: llc -mtriple=wasm32-unknown-unknown -mattr=+multivalue -wasm-emit-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" diff --git a/llvm/test/CodeGen/WebAssembly/multivalue-stackify.ll b/llvm/test/CodeGen/WebAssembly/multivalue-stackify.ll index 52a8c686824d3..f4f93ac2f30ce 100644 --- a/llvm/test/CodeGen/WebAssembly/multivalue-stackify.ll +++ b/llvm/test/CodeGen/WebAssembly/multivalue-stackify.ll @@ -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 | FileCheck %s +; RUN: llc < %s -verify-machineinstrs -mattr=+multivalue -wasm-emit-multivalue | FileCheck %s ; Test that the multivalue stackification works diff --git a/llvm/test/CodeGen/WebAssembly/multivalue.ll b/llvm/test/CodeGen/WebAssembly/multivalue.ll index 675009c8f3e54..846691e5ff0cd 100644 --- a/llvm/test/CodeGen/WebAssembly/multivalue.ll +++ b/llvm/test/CodeGen/WebAssembly/multivalue.ll @@ -1,7 +1,8 @@ -; 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 +; 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 ; Test that the multivalue calls, returns, function types, and block ; types work as expected. @@ -19,6 +20,7 @@ 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 } } diff --git a/llvm/test/CodeGen/WebAssembly/multivalue_libcall.ll b/llvm/test/CodeGen/WebAssembly/multivalue_libcall.ll index 47c5ae7b457dd..7bf37b59353ad 100644 --- a/llvm/test/CodeGen/WebAssembly/multivalue_libcall.ll +++ b/llvm/test/CodeGen/WebAssembly/multivalue_libcall.ll @@ -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 | FileCheck %s --check-prefix=MULTIVALUE +; RUN: llc < %s -verify-machineinstrs -mcpu=mvp -mattr=+multivalue -wasm-emit-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 From b7f3f52821fd8adbb4585218b054d6264b91f735 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Fri, 23 Feb 2024 00:28:17 +0000 Subject: [PATCH 2/2] clang-format --- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp index 8814d0570dc41..3120b6b67906e 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp @@ -58,10 +58,10 @@ static cl::opt WasmDisableFixIrreducibleControlFlowPass( // 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 WasmEmitMultiValue( - "wasm-emit-multivalue", cl::Hidden, - cl::desc("WebAssembly: Emit multivalue in the backend"), - cl::init(false)); +cl::opt + 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.