[PATCH] D41226: LiveDebugValues spill recognition expasnsion

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

Thanks! Couple of comments inline.



================
Comment at: lib/CodeGen/LiveDebugValues.cpp:435
+    if (MO.isReg() && MO.isUse()) {
+      if(MO.isKill()) {
+        Reg = MO.getReg();
----------------
please run your patch through clang-format to fix up the indentation


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:441
+        // Check wether next instruction kills the spilled register.
+        MachineBasicBlock::const_iterator MII = &MI;
+        MII++;
----------------
perhaps write this as `auto NextI = std::next(MI);` ?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:448
+            unsigned RegNext = MONext.getReg();
+            if(RegNext == MO.getReg()){
+              Reg = RegNext;
----------------
This looks like it is *almost* the same code as on line 434. Would it make sense to factor this into a separate function or lambda?


================
Comment at: test/DebugInfo/X86/kill-after-spill.ll:1
+; RUN: llc < %s | FileCheck %s
+;
----------------
This test is really long, could it be further reduced?


================
Comment at: test/DebugInfo/X86/kill-after-spill.ll:4
+; This test is used to acknowledge situation when spill register is killed
+; in istruction after the spill occurs
+
----------------
For Machine passes, it is usually preferable to write MIR tests.

To convert this into MIR, run

`llc -stop-before=livedebugvalues -o -`

and then in the test do

`RUN: llc -run-pass=livedebugvalues`


https://reviews.llvm.org/D41226