From d198c75e5ae0415bf457f0d1a46940ee758c6b0d Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Sat, 17 Dec 2022 16:33:22 -0800 Subject: [PATCH] [WebAssembly][LiveDebugValues] Handle target index defs This adds the missing handling for defs for target index operands, as is already done for registers. There are two kinds of target indices: local indices and stack operands. - Locals are something similar to registers in Wasm-land. For local indices, we can check for local-defining instructions (`local.set` or `local.tee`). - Wasm is a stack machine, so we have values in certain Wasm value stack location, which change when Wasm instructions produce or consume values. So basically any value-producing instrucion, i.e., instruction with defs, can change values in the Wasm stack. But I think we don't need to worry about this here, because `WebAssemblyDebugFixup`, which runs right before this analysis, makes sure to insert terminating `DBG_VALUE $noreg` instructions whenever a stack value gets popped. After `WebAssemblyDebugFixup`, there shouldn't be any `DBG_VALUE`s for stack operands that don't have a terminating `DBG_VALUE $noreg` within the same BB. So this CL only works on `DBG_VALUE`s for locals. When we encounter a `local.set` or `local.tee` instructions, we delete `DBG_VALUE`s for those target index locations from the open range set, so they will not be availble in `OutLocs`. For example, ``` bb.0: successors: %bb.1 DBG_VALUE target-index(wasm-local) + 2, $noreg, "var", ... ... local.set 2 ... bb.1: ; predecessors: %bb.0 ; We shouldn't add `DBG_VALUE target (wasm-local) + 2 here because ; it was killed by 'local.set' in bb.0 ``` After disabling register coalescing at -O1, the average PC ranges covered for Emscripten core benchmarks is currently 20.6% in the LLVM tot. After applying D138943 and this CL, the coverage goes up to 57%. This also enables LiveDebugValues analysis in the Wasm pipeline by default. Reviewed By: dschuff, jmorse Differential Revision: https://reviews.llvm.org/D140373 --- llvm/include/llvm/CodeGen/TargetInstrInfo.h | 9 +++ .../CodeGen/LiveDebugValues/VarLocBasedImpl.cpp | 84 +++++++++++++++++++--- .../MCTargetDesc/WebAssemblyMCTargetDesc.h | 66 +++++++++++++++++ .../Target/WebAssembly/WebAssemblyInstrInfo.cpp | 27 +++++++ llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.h | 3 + .../WebAssembly/WebAssemblyTargetMachine.cpp | 1 - .../DebugInfo/WebAssembly/live-debug-values.mir | 52 ++++++++++++++ 7 files changed, 230 insertions(+), 12 deletions(-) diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h index 1f95b63..b15e958 100644 --- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h +++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h @@ -2061,6 +2061,15 @@ public: return InstructionUniformity::Default; } + /// Returns true if the given \p MI defines a TargetIndex operand that can be + /// tracked by their offset, can have values, and can have debug info + /// associated with it. If so, sets \p Index and \p Offset of the target index + /// operand. + virtual bool isExplicitTargetIndexDef(const MachineInstr &MI, int &Index, + int64_t &Offset) const { + return false; + } + private: mutable std::unique_ptr Formatter; unsigned CallFrameSetupOpcode, CallFrameDestroyOpcode; diff --git a/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp index 062b5f5..ce5e8ad 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp @@ -10,12 +10,13 @@ /// /// LiveDebugValues is an optimistic "available expressions" dataflow /// algorithm. The set of expressions is the set of machine locations -/// (registers, spill slots, constants) that a variable fragment might be -/// located, qualified by a DIExpression and indirect-ness flag, while each -/// variable is identified by a DebugVariable object. The availability of an -/// expression begins when a DBG_VALUE instruction specifies the location of a -/// DebugVariable, and continues until that location is clobbered or -/// re-specified by a different DBG_VALUE for the same DebugVariable. +/// (registers, spill slots, constants, and target indices) that a variable +/// fragment might be located, qualified by a DIExpression and indirect-ness +/// flag, while each variable is identified by a DebugVariable object. The +/// availability of an expression begins when a DBG_VALUE instruction specifies +/// the location of a DebugVariable, and continues until that location is +/// clobbered or re-specified by a different DBG_VALUE for the same +/// DebugVariable. /// /// The output of LiveDebugValues is additional DBG_VALUE instructions, /// placed to extend variable locations as far they're available. This file @@ -230,6 +231,14 @@ struct LocIndex { static constexpr u32_location_t kEntryValueBackupLocation = kFirstInvalidRegLocation + 1; + /// A special location reserved for VarLocs with locations of kind + /// WasmLocKind. + /// TODO Placing all Wasm target index locations in this single kWasmLocation + /// may cause slowdown in compilation time in very large functions. Consider + /// giving a each target index/offset pair its own u32_location_t if this + /// becomes a problem. + static constexpr u32_location_t kWasmLocation = kFirstInvalidRegLocation + 2; + LocIndex(u32_location_t Location, u32_index_t Index) : Location(Location), Index(Index) {} @@ -301,7 +310,12 @@ private: // Target indices used for wasm-specific locations. struct WasmLoc { - int Index; // One of TargetIndex values defined in WebAssembly.h + // One of TargetIndex values defined in WebAssembly.h. We deal with + // local-related TargetIndex in this analysis (TI_LOCAL and + // TI_LOCAL_INDIRECT). Stack operands (TI_OPERAND_STACK) will be handled + // separately WebAssemblyDebugFixup pass, and we don't associate debug + // info with values in global operands (TI_GLOBAL_RELOC) at the moment. + int Index; int64_t Offset; bool operator==(const WasmLoc &Other) const { return Index == Other.Index && Offset == Other.Offset; @@ -673,6 +687,21 @@ private: llvm_unreachable("Could not find given SpillLoc in Locs"); } + bool containsWasmLocs() const { + return any_of(Locs, [](VarLoc::MachineLoc ML) { + return ML.Kind == VarLoc::MachineLocKind::WasmLocKind; + }); + } + + /// If this variable is described in whole or part by \p WasmLocation, + /// return true. + bool usesWasmLoc(WasmLoc WasmLocation) const { + MachineLoc WasmML; + WasmML.Kind = MachineLocKind::WasmLocKind; + WasmML.Value.WasmLocation = WasmLocation; + return is_contained(Locs, WasmML); + } + /// Determine whether the lexical scope of this value's debug location /// dominates MBB. bool dominates(LexicalScopes &LS, MachineBasicBlock &MBB) const { @@ -784,10 +813,10 @@ private: return RegNo < LocIndex::kFirstInvalidRegLocation; }) && "Physreg out of range?"); - if (VL.containsSpillLocs()) { - LocIndex::u32_location_t Loc = LocIndex::kSpillLocation; - Locations.push_back(Loc); - } + if (VL.containsSpillLocs()) + Locations.push_back(LocIndex::kSpillLocation); + if (VL.containsWasmLocs()) + Locations.push_back(LocIndex::kWasmLocation); } else if (VL.EVKind != VarLoc::EntryValueLocKind::EntryValueKind) { LocIndex::u32_location_t Loc = LocIndex::kEntryValueBackupLocation; Locations.push_back(Loc); @@ -940,6 +969,12 @@ private: return LocIndex::indexRangeForLocation( getVarLocs(), LocIndex::kEntryValueBackupLocation); } + + /// Get all set IDs for VarLocs with MLs of kind WasmLocKind. + auto getWasmVarLocs() const { + return LocIndex::indexRangeForLocation(getVarLocs(), + LocIndex::kWasmLocation); + } }; /// Collect all VarLoc IDs from \p CollectFrom for VarLocs with MLs of kind @@ -1026,6 +1061,8 @@ private: VarLocMap &VarLocIDs, InstToEntryLocMap &EntryValTransfers, RegDefToInstMap &RegSetInstrs); + void transferWasmDef(MachineInstr &MI, OpenRangesSet &OpenRanges, + VarLocMap &VarLocIDs); bool transferTerminator(MachineBasicBlock *MBB, OpenRangesSet &OpenRanges, VarLocInMBB &OutLocs, const VarLocMap &VarLocIDs); @@ -1606,6 +1643,30 @@ void VarLocBasedLDV::transferRegisterDef(MachineInstr &MI, } } +void VarLocBasedLDV::transferWasmDef(MachineInstr &MI, + OpenRangesSet &OpenRanges, + VarLocMap &VarLocIDs) { + // If this is not a Wasm local.set or local.tee, which sets local values, + // return. + int Index; + int64_t Offset; + if (!TII->isExplicitTargetIndexDef(MI, Index, Offset)) + return; + + // Find the target indices killed by MI, and delete those variable locations + // from the open range. + VarLocsInRange KillSet; + VarLoc::WasmLoc Loc{Index, Offset}; + for (uint64_t ID : OpenRanges.getWasmVarLocs()) { + LocIndex Idx = LocIndex::fromRawInteger(ID); + const VarLoc &VL = VarLocIDs[Idx]; + assert(VL.containsWasmLocs() && "Broken VarLocSet?"); + if (VL.usesWasmLoc(Loc)) + KillSet.insert(ID); + } + OpenRanges.erase(KillSet, VarLocIDs, LocIndex::kWasmLocation); +} + bool VarLocBasedLDV::isSpillInstruction(const MachineInstr &MI, MachineFunction *MF) { // TODO: Handle multiple stores folded into one. @@ -1944,6 +2005,7 @@ void VarLocBasedLDV::process(MachineInstr &MI, OpenRangesSet &OpenRanges, RegSetInstrs); transferRegisterDef(MI, OpenRanges, VarLocIDs, EntryValTransfers, RegSetInstrs); + transferWasmDef(MI, OpenRanges, VarLocIDs); transferRegisterCopy(MI, OpenRanges, VarLocIDs, Transfers); transferSpillOrRestoreInst(MI, OpenRanges, VarLocIDs, Transfers); } diff --git a/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h b/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h index b5b1220..476955e 100644 --- a/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h +++ b/llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h @@ -416,6 +416,72 @@ inline bool isCatch(unsigned Opc) { } } +inline bool isLocalGet(unsigned Opc) { + switch (Opc) { + case WebAssembly::LOCAL_GET_I32: + case WebAssembly::LOCAL_GET_I32_S: + case WebAssembly::LOCAL_GET_I64: + case WebAssembly::LOCAL_GET_I64_S: + case WebAssembly::LOCAL_GET_F32: + case WebAssembly::LOCAL_GET_F32_S: + case WebAssembly::LOCAL_GET_F64: + case WebAssembly::LOCAL_GET_F64_S: + case WebAssembly::LOCAL_GET_V128: + case WebAssembly::LOCAL_GET_V128_S: + case WebAssembly::LOCAL_GET_FUNCREF: + case WebAssembly::LOCAL_GET_FUNCREF_S: + case WebAssembly::LOCAL_GET_EXTERNREF: + case WebAssembly::LOCAL_GET_EXTERNREF_S: + return true; + default: + return false; + } +} + +inline bool isLocalSet(unsigned Opc) { + switch (Opc) { + case WebAssembly::LOCAL_SET_I32: + case WebAssembly::LOCAL_SET_I32_S: + case WebAssembly::LOCAL_SET_I64: + case WebAssembly::LOCAL_SET_I64_S: + case WebAssembly::LOCAL_SET_F32: + case WebAssembly::LOCAL_SET_F32_S: + case WebAssembly::LOCAL_SET_F64: + case WebAssembly::LOCAL_SET_F64_S: + case WebAssembly::LOCAL_SET_V128: + case WebAssembly::LOCAL_SET_V128_S: + case WebAssembly::LOCAL_SET_FUNCREF: + case WebAssembly::LOCAL_SET_FUNCREF_S: + case WebAssembly::LOCAL_SET_EXTERNREF: + case WebAssembly::LOCAL_SET_EXTERNREF_S: + return true; + default: + return false; + } +} + +inline bool isLocalTee(unsigned Opc) { + switch (Opc) { + case WebAssembly::LOCAL_TEE_I32: + case WebAssembly::LOCAL_TEE_I32_S: + case WebAssembly::LOCAL_TEE_I64: + case WebAssembly::LOCAL_TEE_I64_S: + case WebAssembly::LOCAL_TEE_F32: + case WebAssembly::LOCAL_TEE_F32_S: + case WebAssembly::LOCAL_TEE_F64: + case WebAssembly::LOCAL_TEE_F64_S: + case WebAssembly::LOCAL_TEE_V128: + case WebAssembly::LOCAL_TEE_V128_S: + case WebAssembly::LOCAL_TEE_FUNCREF: + case WebAssembly::LOCAL_TEE_FUNCREF_S: + case WebAssembly::LOCAL_TEE_EXTERNREF: + case WebAssembly::LOCAL_TEE_EXTERNREF_S: + return true; + default: + return false; + } +} + } // end namespace WebAssembly } // end namespace llvm diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp index cfa693e..b2dd656 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp @@ -204,3 +204,30 @@ const MachineOperand & WebAssemblyInstrInfo::getCalleeOperand(const MachineInstr &MI) const { return WebAssembly::getCalleeOp(MI); } + +// This returns true when the instruction defines a value of a TargetIndex +// operand that can be tracked by offsets. For Wasm, this returns true for only +// local.set/local.tees. This is currently used by LiveDebugValues analysis. +// +// These are not included: +// - In theory we need to add global.set here too, but we don't have global +// indices at this point because they are relocatable and we address them by +// names until linking, so we don't have 'offsets' (which are used to store +// local/global indices) to deal with in LiveDebugValues. And we don't +// associate debug info in values in globals anyway. +// - All other value-producing instructions, i.e. instructions with defs, can +// define values in the Wasm stack, which is represented by TI_OPERAND_STACK +// TargetIndex. But they don't have offset info within the instruction itself, +// and debug info analysis for them is handled separately in +// WebAssemblyDebugFixup pass, so we don't worry about them here. +bool WebAssemblyInstrInfo::isExplicitTargetIndexDef(const MachineInstr &MI, + int &Index, + int64_t &Offset) const { + unsigned Opc = MI.getOpcode(); + if (WebAssembly::isLocalSet(Opc) || WebAssembly::isLocalTee(Opc)) { + Index = WebAssembly::TI_LOCAL; + Offset = MI.explicit_uses().begin()->getImm(); + return true; + } + return false; +} diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.h b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.h index 29d700b..c1e1a79 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.h +++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.h @@ -69,6 +69,9 @@ public: getSerializableTargetIndices() const override; const MachineOperand &getCalleeOperand(const MachineInstr &MI) const override; + + bool isExplicitTargetIndexDef(const MachineInstr &MI, int &Index, + int64_t &Offset) const override; }; } // end namespace llvm diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp index 6bab767..630c786 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp @@ -517,7 +517,6 @@ void WebAssemblyPassConfig::addPostRegAlloc() { disablePass(&PostRASchedulerID); disablePass(&FuncletLayoutID); disablePass(&StackMapLivenessID); - disablePass(&LiveDebugValuesID); disablePass(&PatchableFunctionID); disablePass(&ShrinkWrapID); diff --git a/llvm/test/DebugInfo/WebAssembly/live-debug-values.mir b/llvm/test/DebugInfo/WebAssembly/live-debug-values.mir index 6a854a3..de25c46 100644 --- a/llvm/test/DebugInfo/WebAssembly/live-debug-values.mir +++ b/llvm/test/DebugInfo/WebAssembly/live-debug-values.mir @@ -22,6 +22,13 @@ ret void } + define void @test_target_index_defs() !dbg !21 { + call void @llvm.dbg.value(metadata i32 0, metadata !22, metadata !DIExpression()), !dbg !23 + call void @llvm.dbg.value(metadata i32 0, metadata !24, metadata !DIExpression()), !dbg !23 + call void @llvm.dbg.value(metadata i32 0, metadata !25, metadata !DIExpression()), !dbg !23 + ret void + } + declare void @llvm.dbg.value(metadata, metadata, metadata) !llvm.dbg.cu = !{!0} @@ -59,6 +66,11 @@ ; CHECK: ![[T1_VAR3:[0-9]+]] = !DILocalVariable(name: "var3", scope: ![[T1_SP]] !20 = !DILocalVariable(name: "var4", scope: !14, file: !1, line: 11, type: !8) ; CHECK: ![[T1_VAR4:[0-9]+]] = !DILocalVariable(name: "var4", scope: ![[T1_SP]] + !21 = distinct !DISubprogram(name: "test_target_index_defs", scope: !1, file: !1, line: 10, type: !6, scopeLine: 1, unit: !0) + !22 = !DILocalVariable(name: "var0", scope: !21, file: !1, line: 20, type: !8) + !23 = !DILocation(line: 10, scope: !21) + !24 = !DILocalVariable(name: "var1", scope: !21, file: !1, line: 20, type: !8) + !25 = !DILocalVariable(name: "var2", scope: !21, file: !1, line: 20, type: !8) ... --- @@ -179,3 +191,43 @@ body: | ; predecessors: %bb.1, %bb.2 RETURN implicit-def dead $arguments ... + +--- +# bb.0 defines DBG_VALUEs for local index 2 and 3. The DBG_VALUE for local index +# 2 is killed in bb.1 due to 'local.tee 2' there, and DBG_VALUE for local +# index 3 is killed in bb.2 because there is 'local.set 3' there. As a result, +# bb.3 shoudln't have any additional DBG_VALUEs added for those locals. +# CHECK-LABEL: name: test_target_index_defs +name: test_target_index_defs +liveins: + - { reg: '$arguments' } +body: | + bb.0: + successors: %bb.1, %bb.2 + liveins: $arguments + DBG_VALUE target-index(wasm-local) + 2, $noreg, !22, !DIExpression(), debug-location !23 + DBG_VALUE target-index(wasm-local) + 3, $noreg, !24, !DIExpression(), debug-location !23 + %0:i32 = CONST_I32 1, implicit-def $arguments + BR_IF %bb.2, %0:i32, implicit-def $arguments + + bb.1: + ; predecessors: %bb.0 + successors: %bb.3 + %0:i32 = CONST_I32 1, implicit-def $arguments + %1:i32 = LOCAL_TEE_I32 2, %0:i32, implicit-def $arguments + DROP_I32 killed %1:i32, implicit-def $arguments + BR %bb.3, implicit-def $arguments + + bb.2: + ; predecessors: %bb.0 + successors: %bb.3 + %0:i32 = CONST_I32 1, implicit-def $arguments + LOCAL_SET_I32 3, %0:i32, implicit-def $arguments + BR %bb.3, implicit-def $arguments + + ; CHECK: bb.3: + ; CHECK-NOT: DBG_VALUE target-index(wasm-local){{.*}} + bb.3: + ; predecessors: %bb.1, %bb.2 + RETURN implicit-def dead $arguments +... -- 2.7.4