[PATCH] D56401: [WebAssembly] Fix updating/moving DBG_VALUEs in RegStackify

Yury Delendik via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 10 11:27:51 PST 2019


yurydelendik marked 6 inline comments as done.
yurydelendik added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyRegStackify.cpp:43
 
+class AdjacentDebugValues {
+  MachineInstr *Def;
----------------
aheejin wrote:
> It's still not in the anonymous namespace I think..?
> Also, not sure about the name `AdjacentDebugValues`.. which sounds like there are also non-adjacent DBG_VALUE instructions.
As I understand DBG_VALUE concept, there can be DBG_VALUE without defining MI. Changed name to `MachineInstrAdjacentDebugValues`


================
Comment at: lib/Target/WebAssembly/WebAssemblyRegStackify.cpp:68
+  void clone(MachineInstr *Insert, unsigned NewReg,
+             const WebAssemblyInstrInfo *TII) {
+    MachineBasicBlock *MBB = Insert->getParent();
----------------
aheejin wrote:
> I don't think we need to pass `TII` here.. It looks like you can call `MachineFunction::CloneMachineInstrBundle` to clone an instruction, which `WebAssemblyInstrInfo::duplicate` is calling anyway?
I'm not sure if `CloneMachineInstrBundle` will be a right choice here: if it's applicable to DBG_VALUE, and, also, `RematerializeCheapDef` tried to clone origin MI via TII->reMaterialize, probably breaking any bundle. Use `CloneMachineInstr` for now.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56401





More information about the llvm-commits mailing list