[llvm] d198c75 - [WebAssembly][LiveDebugValues] Handle target index defs

Heejin Ahn via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 10 09:57:17 PST 2023


Author: Heejin Ahn
Date: 2023-01-10T09:56:25-08:00
New Revision: d198c75e5ae0415bf457f0d1a46940ee758c6b0d

URL: https://github.com/llvm/llvm-project/commit/d198c75e5ae0415bf457f0d1a46940ee758c6b0d
DIFF: https://github.com/llvm/llvm-project/commit/d198c75e5ae0415bf457f0d1a46940ee758c6b0d.diff

LOG: [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

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/TargetInstrInfo.h
    llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp
    llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
    llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp
    llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.h
    llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
    llvm/test/DebugInfo/WebAssembly/live-debug-values.mir

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 1f95b630e26b7..b15e95831c30f 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -2061,6 +2061,15 @@ class TargetInstrInfo : public MCInstrInfo {
     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<MIRFormatter> Formatter;
   unsigned CallFrameSetupOpcode, CallFrameDestroyOpcode;

diff  --git a/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp
index 062b5f5e2a91d..ce5e8ad3047f7 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 
diff erent 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 
diff erent 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 @@ class VarLocBasedLDV : public LDVImpl {
 
     // 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 @@ class VarLocBasedLDV : public LDVImpl {
       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 @@ class VarLocBasedLDV : public LDVImpl {
                         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 @@ class VarLocBasedLDV : public LDVImpl {
       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 @@ class VarLocBasedLDV : public LDVImpl {
                            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 b5b12200505b4..476955e434f28 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 cfa693e366085..b2dd656ccdda3 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 29d700bdf83f7..c1e1a790c60e2 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.h
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.h
@@ -69,6 +69,9 @@ class WebAssemblyInstrInfo final : public WebAssemblyGenInstrInfo {
   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 6bab7676a1788..630c786a3dc78 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 6a854a3d0bf1e..de25c46aecae7 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
+...


        


More information about the llvm-commits mailing list