[PATCH] D56151: [DebugInfo] PR40010: Avoid register coalesing altering DBG_VALUE valuations

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: test/CodeGen/X86/pr40010.mir:81
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
----------------
I think you can remove all the false initializations here (keep `tracksRegLiveness: true`).

See: https://llvm.org/docs/MIRLangRef.html#simplifying-mir-files


================
Comment at: test/CodeGen/X86/pr40010.mir:88
+hasWinCFI:       false
+registers:
+  - { id: 0, class: gr32, preferred-register: '' }
----------------
I think the `registers:` section can be removed (these mapping are given by the MIR below, right?
And `liveins`, `fixedStack`, `stack`, `constants` can also be removed here, right?

See: https://llvm.org/docs/MIRLangRef.html#simplifying-mir-files


================
Comment at: test/CodeGen/X86/pr40010.mir:114
+  bb.1.start.test1:
+    ; CHECK-LABEL: bb.1.start.test1
+    successors: %bb.2(0x04000000), %bb.1(0x7c000000)
----------------
Maybe easier to read if you put all CHECK:s for the basic block here (consecutive lines)?
Or all checks for the function just before/after the function (might need to use `# CHECK:` instead of `; CHECK:` depending on context.

(I've actually never used `; CHECK:`  within the body like this so I did not know that it works like that.)

I would probably have used something like this:
```
# <description of what the test is supposed to verify>
# 
# CHECK-LABEL: name:            test1
# CHECK: bb.1:
# CHECK:   %7:gr32 = ADD32rr %7
# CHECK:   DBG_VALUE $noreg
# CHECK: bb.2:
```
Where the CHECK-LABEL is supposed to make sure that each subtest is self-contained (do not match with anything from another subtest). This also makes it possible to skip some of the IR (since you do not need the symbolic names for the basic blocks.


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

https://reviews.llvm.org/D56151