[PATCH] D49034: [WebAssembly] Move/clone DBG_VALUE during WebAssemblyRegStackify pass

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added a comment.

It looks DBG_VALUE instruction handling in RegStackify has several problems.

1. `MoveDebugValues` and `UpdateDebugValuesReg` do not seem to consider the case where there are multiple defs to a single register within a BB. Currently they treat all `DBG_VALUE` instruction to a register as a group and move / update them together whenever any one of the definitions of the register in a BB is stackified. A sample test case with multiple defs within a BB that fails is attached in PR40209 <https://bugs.llvm.org/show_bug.cgi?id=40209>.

2-3 are written inline.

I also filed PR40209 <https://bugs.llvm.org/show_bug.cgi?id=40209> with the same contents for easy tracking.



================
Comment at: llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp:486
+    if (MI->isDebugValue() && MI->getParent() == &MBB)
+      Op.setReg(NewReg);
+  }
----------------
Here `Op.setReg(NewReg)` modifies the iterator `MRI.reg_operands(Reg)` while traversing it. So this ends up hitting only the first element and bailing out. This function has problems even without this bug because of the 1 above though.


================
Comment at: llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp:542
+    assert(MI != nullptr);
+    if (MI->isDebugValue() && MI->getParent() == &MBB &&
+        Instrs.find(MI) == Instrs.end())
----------------
This function does not consider a case that an instruction to be rematerialized is in another BB. Even though RegStackify mostly work within a BB, when a def is trivially rematerializable, i.e., `const_i32` instruction, that instruction can be another BB and can be copied to this BB. But `CloneDebugValues` limit the search within this BB by `MI->getParent() == &MBB` clause, so in that case the debug value for the rematerialized instruction will not be cloned. A test case can be made trivially, so I'm not gonna attach an mir file for that, but it should look like this:
```
bb1:
  %3 = const_i32 3 ; This is trivially rematerializable
  ...

bb2:
  ...
  use %3

```


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49034/new/

https://reviews.llvm.org/D49034