[PATCH] D127208: MachineSink debug invariance

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

> Should add a .mir test

+1

This change looks ok to me but FTR I haven't touched the MachineSink pass before.



================
Comment at: llvm/lib/CodeGen/MachineSink.cpp:271
 
+static unsigned sizeNoDbg(const MachineBasicBlock &MBB) {
+  unsigned Size = 0;
----------------
markus wrote:
> Don't we already have some utility function for this?
It looks like we have `.instructionsWithoutDebug()` and `.sizeWithoutDebug()`  for `BasicBlock`  but I'm not sure if there's an equivalent for `MachineBasicBlock`. Maybe it'd be worth adding to  `MachineBasicBlock`?


================
Comment at: llvm/lib/CodeGen/MachineSink.cpp:274
+  for (auto &MI : MBB) {
+    if (MI.isDebugInstr())
+      continue;
----------------
Should this be `.isMetaInstr()` (rather than `.isDebugInstr()`) to cover other non-debug that do not correspond to executable instructions?


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

https://reviews.llvm.org/D127208