[PATCH] D146860: [WebAssembly] Do nothing when sinking to same place

[Heejin Ahn via Phabricator via llvm-commits]

aheejin created this revision.
aheejin added a reviewer: dschuff.
Herald added subscribers: pmatos, asb, wingo, ecnelises, sunfish, hiraditya, jgravelle-google, sbc100.
Herald added a project: All.
aheejin requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

In `DebugValueManager`, if a `Def` is requested to be sunk to the same
place (i.e., `Insert` is right after `Def`, not counting `DBG_VALUE`s)
currently we still do the sink. This can result in unnecessary creation
of `DBG_VALUE $noreg`. See comments for details. This CL detects this
case and do nothing and return, so we don't end up creating unnecessary
undef `DBG_VALUE`s.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146860

Files:
  llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.h
  llvm/test/DebugInfo/WebAssembly/dbg-value-reg-stackify.mir


Index: llvm/test/DebugInfo/WebAssembly/dbg-value-reg-stackify.mir
===================================================================
--- llvm/test/DebugInfo/WebAssembly/dbg-value-reg-stackify.mir
+++ llvm/test/DebugInfo/WebAssembly/dbg-value-reg-stackify.mir
@@ -191,9 +191,7 @@
     CALL @use, %0:i32, implicit-def $arguments
     RETURN implicit-def $arguments
 
-  ; CHECK:      DBG_VALUE $noreg, $noreg, ![[VAR_A]], !DIExpression()
-  ; CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[VAR_B]], !DIExpression()
-  ; CHECK-NEXT: %0:i32 = CONST_I32 1, implicit-def $arguments, implicit-def $value_stack, implicit $value_stack
+  ; CHECK:      %0:i32 = CONST_I32 1, implicit-def $arguments, implicit-def $value_stack, implicit $value_stack
   ; CHECK-NEXT: DBG_VALUE %0, $noreg, ![[VAR_A]], !DIExpression()
   ; CHECK-NEXT: DBG_VALUE %0, $noreg, ![[VAR_B]], !DIExpression()
   ; CHECK-NEXT: CALL @use, %0, implicit-def $arguments, implicit-def $value_stack, implicit $value_stack
Index: llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.h
===================================================================
--- llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.h
+++ llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.h
@@ -31,6 +31,7 @@
   Register CurrentReg;
   SmallVector<MachineInstr *>
   getSinkableDebugValues(MachineInstr *Insert) const;
+  bool isInsertSamePlace(MachineInstr *Insert) const;
 
 public:
   WebAssemblyDebugValueManager(MachineInstr *Def);
Index: llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp
===================================================================
--- llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp
@@ -210,6 +210,22 @@
   return SinkableDbgValues;
 }
 
+// Returns true if the insertion point is the same as the current place.
+// Following DBG_VALUEs for 'Def' are ignored.
+bool WebAssemblyDebugValueManager::isInsertSamePlace(
+    MachineInstr *Insert) const {
+  if (Def->getParent() != Insert->getParent())
+    return false;
+  for (MachineBasicBlock::iterator MI = std::next(Def->getIterator()),
+                                   ME = Insert;
+       MI != ME; ++MI) {
+    if (std::find(DbgValues.begin(), DbgValues.end(), MI) == DbgValues.end()) {
+      return false;
+    }
+  }
+  return true;
+}
+
 // Sink 'Def', and also sink its eligible DBG_VALUEs to the place before
 // 'Insert'. Convert the original DBG_VALUEs into undefs.
 //
@@ -221,6 +237,23 @@
 // This DebugValueManager's new Def and DbgValues will be updated to the newly
 // sinked Def + DBG_VALUEs.
 void WebAssemblyDebugValueManager::sink(MachineInstr *Insert) {
+  // In case Def is requested to be sunk to
+  // the same place, we don't need to do anything. If we actually do the sink,
+  // it will create unnecessary undef DBG_VALUEs. For example, if the original
+  // code is:
+  //   %0 = someinst           // Def
+  //   DBG_VALUE %0, ...
+  //   %1 = anotherinst        // Insert
+  //
+  // If we actually sink %0 and the following DBG_VALUE and setting the original
+  // DBG_VALUE undef, the result will be:
+  //   DBG_VALUE %noreg, ...   // Unnecessary!
+  //   %0 = someinst           // Def
+  //   DBG_VALUE %0, ...
+  //   %1 = anotherinst        // Insert
+  if (isInsertSamePlace(Insert))
+    return;
+
   MachineBasicBlock *MBB = Insert->getParent();
   MachineFunction *MF = MBB->getParent();
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D146860.508263.patch
Type: text/x-patch
Size: 3450 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230325/6038c83a/attachment.bin>