From: Andy Wingo Date: Fri, 28 May 2021 10:42:12 +0000 (+0200) Subject: Revert "[WebAssembly][CodeGen] IR support for WebAssembly local variables" X-Git-Tag: llvmorg-14-init~5353 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ca5f07f8c4bc96d16ed1992b810aa3897df157f2;p=platform%2Fupstream%2Fllvm.git Revert "[WebAssembly][CodeGen] IR support for WebAssembly local variables" This reverts commit 00ecf18979e3326b3afee8af3dc701c53ffdc93f, as it broke the AMDGPU build. Will reland later with a fix. --- diff --git a/llvm/include/llvm/CodeGen/MIRYamlMapping.h b/llvm/include/llvm/CodeGen/MIRYamlMapping.h index 9d3878f..a7bbe7d 100644 --- a/llvm/include/llvm/CodeGen/MIRYamlMapping.h +++ b/llvm/include/llvm/CodeGen/MIRYamlMapping.h @@ -348,7 +348,6 @@ struct ScalarEnumerationTraits { IO.enumCase(ID, "default", TargetStackID::Default); IO.enumCase(ID, "sgpr-spill", TargetStackID::SGPRSpill); IO.enumCase(ID, "scalable-vector", TargetStackID::ScalableVector); - IO.enumCase(ID, "wasm-local", TargetStackID::WasmLocal); IO.enumCase(ID, "noalloc", TargetStackID::NoAlloc); } }; diff --git a/llvm/include/llvm/CodeGen/TargetFrameLowering.h b/llvm/include/llvm/CodeGen/TargetFrameLowering.h index 69b4916..59dfe98 100644 --- a/llvm/include/llvm/CodeGen/TargetFrameLowering.h +++ b/llvm/include/llvm/CodeGen/TargetFrameLowering.h @@ -24,13 +24,12 @@ namespace llvm { class RegScavenger; namespace TargetStackID { -enum Value { - Default = 0, - SGPRSpill = 1, - ScalableVector = 2, - WasmLocal = 3, - NoAlloc = 255 -}; + enum Value { + Default = 0, + SGPRSpill = 1, + ScalableVector = 2, + NoAlloc = 255 + }; } /// Information about stack frame layout on the target. It holds the direction diff --git a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp index 60e3d1a..824d336 100644 --- a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp +++ b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp @@ -13,12 +13,8 @@ #include "WebAssemblyUtilities.h" #include "WebAssemblyMachineFunctionInfo.h" -#include "WebAssemblySubtarget.h" -#include "llvm/CodeGen/Analysis.h" -#include "llvm/CodeGen/MachineFrameInfo.h" #include "llvm/CodeGen/MachineInstr.h" #include "llvm/CodeGen/MachineLoopInfo.h" -#include "llvm/IR/Instructions.h" #include "llvm/MC/MCContext.h" using namespace llvm; @@ -28,50 +24,6 @@ const char *const WebAssembly::StdTerminateFn = "_ZSt9terminatev"; const char *const WebAssembly::PersonalityWrapperFn = "_Unwind_Wasm_CallPersonality"; -// In an ideal world, when objects are added to the MachineFrameInfo by -// FunctionLoweringInfo::set, we could somehow hook into target-specific code to -// ensure they are assigned the right stack ID. However there isn't a hook that -// runs between then and DAG building time, though, so instead we hoist stack -// objects lazily when they are first used, and comprehensively after the DAG is -// built via the PreprocessISelDAG hook, called by the -// SelectionDAGISel::runOnMachineFunction. We have to do it in two places -// because we want to do it while building the selection DAG for uses of alloca, -// but not all alloca instructions are used so we have to follow up afterwards. -Optional WebAssembly::getLocalForStackObject(MachineFunction &MF, - int FrameIndex) { - auto &MFI = MF.getFrameInfo(); - - // If already hoisted to a local, done. - if (MFI.getStackID(FrameIndex) == TargetStackID::WasmLocal) - return static_cast(MFI.getObjectOffset(FrameIndex)); - - // If not allocated in the object address space, this object will be in - // linear memory. - const AllocaInst *AI = MFI.getObjectAllocation(FrameIndex); - if (!AI || !isWasmVarAddressSpace(AI->getType()->getAddressSpace())) - return None; - - // Otherwise, allocate this object in the named value stack, outside of linear - // memory. - SmallVector ValueVTs; - const WebAssemblyTargetLowering &TLI = - *MF.getSubtarget().getTargetLowering(); - WebAssemblyFunctionInfo *FuncInfo = MF.getInfo(); - ComputeValueVTs(TLI, MF.getDataLayout(), AI->getAllocatedType(), ValueVTs); - MFI.setStackID(FrameIndex, TargetStackID::WasmLocal); - // Abuse SP offset to record the index of the first local in the object. - unsigned Local = FuncInfo->getParams().size() + FuncInfo->getLocals().size(); - MFI.setObjectOffset(FrameIndex, Local); - // Allocate WebAssembly locals for each non-aggregate component of the - // allocation. - for (EVT ValueVT : ValueVTs) - FuncInfo->addLocal(ValueVT.getSimpleVT()); - // Abuse object size to record number of WebAssembly locals allocated to - // this object. - MFI.setObjectSize(FrameIndex, ValueVTs.size()); - return static_cast(Local); -} - /// Test whether MI is a child of some other node in an expression tree. bool WebAssembly::isChild(const MachineInstr &MI, const WebAssemblyFunctionInfo &MFI) { diff --git a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h index ac84e8e..1ec1df5 100644 --- a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h +++ b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h @@ -15,13 +15,10 @@ #ifndef LLVM_LIB_TARGET_WEBASSEMBLY_UTILS_WEBASSEMBLYUTILITIES_H #define LLVM_LIB_TARGET_WEBASSEMBLY_UTILS_WEBASSEMBLYUTILITIES_H -#include "llvm/ADT/Optional.h" - namespace llvm { class MachineBasicBlock; class MachineInstr; -class MachineFunction; class MachineOperand; class MCContext; class MCSymbolWasm; @@ -51,10 +48,6 @@ inline bool isValidAddressSpace(unsigned AS) { return isDefaultAddressSpace(AS) || isWasmVarAddressSpace(AS); } -// Returns the index of the WebAssembly local to which the stack object -// FrameIndex in MF should be allocated, or None. -Optional getLocalForStackObject(MachineFunction &MF, int FrameIndex); - bool isChild(const MachineInstr &MI, const WebAssemblyFunctionInfo &MFI); bool mayThrow(const MachineInstr &MI); diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp index 4a0738d..7ed224d 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp @@ -239,10 +239,8 @@ bool WebAssemblyExplicitLocals::runOnMachineFunction(MachineFunction &MF) { Changed = true; } - // Start assigning local numbers after the last parameter and after any - // already-assigned locals. + // Start assigning local numbers after the last parameter. unsigned CurLocal = static_cast(MFI.getParams().size()); - CurLocal += static_cast(MFI.getLocals().size()); // Precompute the set of registers that are unused, so that we can insert // drops to their defs. diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp index a91fa74..f139e87 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp @@ -314,16 +314,6 @@ void WebAssemblyFrameLowering::emitEpilogue(MachineFunction &MF, writeSPToGlobal(SPReg, MF, MBB, InsertPt, DL); } -bool WebAssemblyFrameLowering::isSupportedStackID( - TargetStackID::Value ID) const { - // Use the Object stack for WebAssembly locals which can only be accessed - // by name, not via an address in linear memory. - if (ID == TargetStackID::WasmLocal) - return true; - - return TargetFrameLowering::isSupportedStackID(ID); -} - TargetFrameLowering::DwarfFrameBase WebAssemblyFrameLowering::getDwarfFrameBase(const MachineFunction &MF) const { DwarfFrameBase Loc; diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h b/llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h index 6d9284d..e16f639 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h +++ b/llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h @@ -43,7 +43,6 @@ public: bool hasFP(const MachineFunction &MF) const override; bool hasReservedCallFrame(const MachineFunction &MF) const override; - bool isSupportedStackID(TargetStackID::Value ID) const override; DwarfFrameBase getDwarfFrameBase(const MachineFunction &MF) const override; bool needsPrologForEH(const MachineFunction &MF) const; diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISD.def b/llvm/lib/Target/WebAssembly/WebAssemblyISD.def index 9e22945..e39e305 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyISD.def +++ b/llvm/lib/Target/WebAssembly/WebAssemblyISD.def @@ -17,8 +17,6 @@ HANDLE_NODETYPE(CALL) HANDLE_NODETYPE(RET_CALL) HANDLE_NODETYPE(RETURN) HANDLE_NODETYPE(ARGUMENT) -HANDLE_NODETYPE(LOCAL_GET) -HANDLE_NODETYPE(LOCAL_SET) // A wrapper node for TargetExternalSymbol, TargetGlobalAddress, and MCSymbol HANDLE_NODETYPE(Wrapper) // A special wapper used in PIC code for __memory_base/__table_base relative diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp index 940c9d5..b9154b0 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp @@ -12,10 +12,8 @@ //===----------------------------------------------------------------------===// #include "MCTargetDesc/WebAssemblyMCTargetDesc.h" -#include "Utils/WebAssemblyUtilities.h" #include "WebAssembly.h" #include "WebAssemblyTargetMachine.h" -#include "llvm/CodeGen/MachineFrameInfo.h" #include "llvm/CodeGen/SelectionDAGISel.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/Function.h" // To access function attributes. @@ -58,8 +56,6 @@ public: return SelectionDAGISel::runOnMachineFunction(MF); } - void PreprocessISelDAG() override; - void Select(SDNode *Node) override; bool SelectInlineAsmMemoryOperand(const SDValue &Op, unsigned ConstraintID, @@ -73,18 +69,6 @@ private: }; } // end anonymous namespace -void WebAssemblyDAGToDAGISel::PreprocessISelDAG() { - // Stack objects that should be allocated to locals are hoisted to WebAssembly - // locals when they are first used. However for those without uses, we hoist - // them here. It would be nice if there were some hook to do this when they - // are added to the MachineFrameInfo, but that's not the case right now. - MachineFrameInfo &FrameInfo = MF->getFrameInfo(); - for (int Idx = 0; Idx < FrameInfo.getObjectIndexEnd(); Idx++) - WebAssembly::getLocalForStackObject(*MF, Idx); - - SelectionDAGISel::PreprocessISelDAG(); -} - void WebAssemblyDAGToDAGISel::Select(SDNode *Node) { // If we have a custom node, we already have selected! if (Node->isMachineOpcode()) { diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp index 909521a..7cd7ab6 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp @@ -1276,15 +1276,6 @@ static bool IsWebAssemblyGlobal(SDValue Op) { return false; } -static Optional IsWebAssemblyLocal(SDValue Op, SelectionDAG &DAG) { - const FrameIndexSDNode *FI = dyn_cast(Op); - if (!FI) - return None; - - auto &MF = DAG.getMachineFunction(); - return WebAssembly::getLocalForStackObject(MF, FI->getIndex()); -} - SDValue WebAssemblyTargetLowering::LowerStore(SDValue Op, SelectionDAG &DAG) const { SDLoc DL(Op); @@ -1304,17 +1295,6 @@ SDValue WebAssemblyTargetLowering::LowerStore(SDValue Op, SN->getMemoryVT(), SN->getMemOperand()); } - if (Optional Local = IsWebAssemblyLocal(Base, DAG)) { - if (!Offset->isUndef()) - report_fatal_error("unexpected offset when storing to webassembly local", - false); - - SDValue Idx = DAG.getTargetConstant(*Local, Base, MVT::i32); - SDVTList Tys = DAG.getVTList(MVT::Other); // The chain. - SDValue Ops[] = {SN->getChain(), Idx, Value}; - return DAG.getNode(WebAssemblyISD::LOCAL_SET, DL, Tys, Ops); - } - return Op; } @@ -1336,20 +1316,6 @@ SDValue WebAssemblyTargetLowering::LowerLoad(SDValue Op, LN->getMemoryVT(), LN->getMemOperand()); } - if (Optional Local = IsWebAssemblyLocal(Base, DAG)) { - if (!Offset->isUndef()) - report_fatal_error( - "unexpected offset when loading from webassembly local", false); - - SDValue Idx = DAG.getTargetConstant(*Local, Base, MVT::i32); - EVT LocalVT = LN->getValueType(0); - SDValue LocalGet = DAG.getNode(WebAssemblyISD::LOCAL_GET, DL, LocalVT, - {LN->getChain(), Idx}); - SDValue Result = DAG.getMergeValues({LocalGet, LN->getChain()}, DL); - assert(Result->getNumValues() == 2 && "Loads must carry a chain!"); - return Result; - } - return Op; } diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td index ac8ad84..72efe7b 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td +++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td @@ -72,8 +72,6 @@ def SDT_WebAssemblyCallSeqEnd : SDCallSeqEnd<[SDTCisVT<0, iPTR>, SDTCisVT<1, iPTR>]>; def SDT_WebAssemblyBrTable : SDTypeProfile<0, -1, [SDTCisPtrTy<0>]>; def SDT_WebAssemblyArgument : SDTypeProfile<1, 1, [SDTCisVT<1, i32>]>; -def SDT_WebAssemblyLocalGet : SDTypeProfile<1, 1, [SDTCisVT<1, i32>]>; -def SDT_WebAssemblyLocalSet : SDTypeProfile<0, 2, [SDTCisVT<0, i32>]>; def SDT_WebAssemblyReturn : SDTypeProfile<0, -1, []>; def SDT_WebAssemblyWrapper : SDTypeProfile<1, 1, [SDTCisSameAs<0, 1>, SDTCisPtrTy<0>]>; @@ -116,12 +114,6 @@ def WebAssemblyglobal_get : def WebAssemblyglobal_set : SDNode<"WebAssemblyISD::GLOBAL_SET", SDT_WebAssemblyGlobalSet, [SDNPHasChain, SDNPMayStore, SDNPMemOperand]>; -def WebAssemblylocal_get : - SDNode<"WebAssemblyISD::LOCAL_GET", SDT_WebAssemblyLocalGet, - [SDNPHasChain, SDNPMayLoad]>; -def WebAssemblylocal_set : - SDNode<"WebAssemblyISD::LOCAL_SET", SDT_WebAssemblyLocalSet, - [SDNPHasChain, SDNPMayStore]>; //===----------------------------------------------------------------------===// // WebAssembly-specific Operands. @@ -340,10 +332,6 @@ multiclass LOCAL { def : Pat<(WebAssemblyglobal_set vt:$src, (WebAssemblywrapper tglobaladdr:$addr)), (!cast("GLOBAL_SET_" # rc) tglobaladdr:$addr, vt:$src)>; - def : Pat<(vt (WebAssemblylocal_get (i32 timm:$local))), - (!cast("LOCAL_GET_" # rc) timm:$local)>; - def : Pat<(WebAssemblylocal_set timm:$local, vt:$src), - (!cast("LOCAL_SET_" # rc) timm:$local, vt:$src)>; } } defm "" : LOCAL; diff --git a/llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll b/llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll deleted file mode 100644 index f5c3630..0000000 --- a/llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll +++ /dev/null @@ -1,22 +0,0 @@ -; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG -; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL - -%f32_cell = type float addrspace(1)* - -; CHECKISEL-LABEL: name: ir_local_f32 -; CHECKISEL: stack: -; CHECKISEL: id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4, -; CHECKISEL-NEXT: stack-id: wasm-local - -; CHECKCG-LABEL: ir_local_f32: -; CHECKCG-NEXT: .functype ir_local_f32 (f32) -> (f32) -; CHECKCG-NEXT: .local f32 -; CHECKCG-NEXT: local.get 0 -; CHECKCG-NEXT: local.set 1 - -define float @ir_local_f32(float %arg) { - %retval = alloca float, addrspace(1) - store float %arg, %f32_cell %retval - %reloaded = load float, %f32_cell %retval - ret float %reloaded -} diff --git a/llvm/test/CodeGen/WebAssembly/ir-locals.ll b/llvm/test/CodeGen/WebAssembly/ir-locals.ll deleted file mode 100644 index a70fcee..0000000 --- a/llvm/test/CodeGen/WebAssembly/ir-locals.ll +++ /dev/null @@ -1,87 +0,0 @@ -; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s - -%i32_cell = type i32 addrspace(1)* -%i64_cell = type i64 addrspace(1)* -%f32_cell = type float addrspace(1)* -%f64_cell = type double addrspace(1)* - -; We have a set of tests in which we set a local and then reload the -; local. If the load immediately follows the set, the DAG combiner will -; infer that the reloaded value is the same value that was set, which -; isn't what we want to test. To inhibit this optimization, we include -; an opaque call between the store and the load. -declare void @inhibit_store_to_load_forwarding() - -define i32 @ir_local_i32(i32 %arg) { - ; CHECK-LABEL: ir_local_i32: - ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32) - %retval = alloca i32, addrspace(1) - ; CHECK-NEXT: .local i32 - store i32 %arg, %i32_cell %retval - ; CHECK-NEXT: local.get 0 - ; CHECK-NEXT: local.set 1 - call void @inhibit_store_to_load_forwarding() - ; CHECK-NEXT: call inhibit_store_to_load_forwarding - %reloaded = load i32, %i32_cell %retval - ; CHECK-NEXT: local.get 1 - ret i32 %reloaded - ; CHECK-NEXT: end_function -} - -define i64 @ir_local_i64(i64 %arg) { - ; CHECK-LABEL: ir_local_i64: - ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64) - %retval = alloca i64, addrspace(1) - ; CHECK-NEXT: .local i64 - store i64 %arg, %i64_cell %retval - ; CHECK-NEXT: local.get 0 - ; CHECK-NEXT: local.set 1 - call void @inhibit_store_to_load_forwarding() - ; CHECK-NEXT: call inhibit_store_to_load_forwarding - %reloaded = load i64, %i64_cell %retval - ; See note in ir_local_i32. - ; CHECK-NEXT: local.get 1 - ret i64 %reloaded - ; CHECK-NEXT: end_function -} - -define float @ir_local_f32(float %arg) { - ; CHECK-LABEL: ir_local_f32: - ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32) - %retval = alloca float, addrspace(1) - ; CHECK-NEXT: .local f32 - store float %arg, %f32_cell %retval - ; CHECK-NEXT: local.get 0 - ; CHECK-NEXT: local.set 1 - call void @inhibit_store_to_load_forwarding() - ; CHECK-NEXT: call inhibit_store_to_load_forwarding - %reloaded = load float, %f32_cell %retval - ; CHECK-NEXT: local.get 1 - ; CHECK-NEXT: end_function - ret float %reloaded -} - -define double @ir_local_f64(double %arg) { - ; CHECK-LABEL: ir_local_f64: - ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64) - %retval = alloca double, addrspace(1) - ; CHECK-NEXT: .local f64 - store double %arg, %f64_cell %retval - ; CHECK-NEXT: local.get 0 - ; CHECK-NEXT: local.set 1 - call void @inhibit_store_to_load_forwarding() - ; CHECK-NEXT: call inhibit_store_to_load_forwarding - %reloaded = load double, %f64_cell %retval - ; CHECK-NEXT: local.get 1 - ; CHECK-NEXT: end_function - ret double %reloaded -} - -define void @ir_unreferenced_local() { - ; CHECK-LABEL: ir_unreferenced_local: - ; CHECK-NEXT: .functype ir_unreferenced_local () -> () - %unused = alloca i32, addrspace(1) - ; CHECK-NEXT: .local i32 - ret void - ; CHECK-NEXT: end_function -}