[PATCH] D106875: [DebugInfo] Attempt to preserve more information during tail duplication

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

This sounds really good; I feel bad about a considerable portion of logic being duplicated here though. Annoyingly as "please rewrite" is, could we instead:

- Give GetValueInMiddleOfBlock a default-false "AllowUnavailable" argument,
- Lambda-ise picking GetValueAtEndOfBlockInternal or the Existing flavour,
- Bail out returning $noreg just before creating new values

Or something like that. IMO, future generations will thank us.



================
Comment at: llvm/include/llvm/CodeGen/MachineSSAUpdater.h:98
   Register GetValueInMiddleOfBlock(MachineBasicBlock *BB);
+  /// As the above, but it will never create new values or instructions - only
+  /// use an existing inserted value; if no such value exists, returns $noreg.
----------------
Nit, newline between comments and preceeding method. IMO "Same as the above" is easier to read, explicit being better than implicit.


================
Comment at: llvm/include/llvm/CodeGen/MachineSSAUpdater.h:100
+  /// use an existing inserted value; if no such value exists, returns $noreg.
+  /// Used for debug values, which are not allowed to effect CodeGen.
+  Register GetExistingValueInMiddleOfBlock(MachineBasicBlock *BB);
----------------



================
Comment at: llvm/lib/CodeGen/TailDuplicator.cpp:233-242
+      UI = MRI->use_begin(VReg);
+      while (UI != MRI->use_end()) {
+        MachineOperand &UseMO = *UI;
+        MachineInstr *UseMI = UseMO.getParent();
+        ++UI;
+        if (!UseMI->isDebugValue())
+          continue;
----------------
Performance opinion: better to store debug uses and re-visit rather than enumerate all the uses again, it'll preserve the same performance characteristics as before this patch.

Also: will not UseMO.setReg invalidate the vreg use list?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106875