[PATCH] D105820: [DebugInfo][InstrRef] Fix a broken substitution method, add test coverage

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added inline comments.


================
Comment at: llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup-2.mir:3
+#
+# Test that various LEA <=> ADD transforms get substitutions applied to them.
+---
----------------
Orlando wrote:
> nit: It loos like this file is just testing LEA => ADD (rather than LEA <=> ADD), or am I missing something?
I guess the comment is misleading; between the two files, all instances of eraseFromParent  (and the substitute calls I put in at those sites) are covered. They're in two different files because we need to cover both i386 and x86_64. I'll fiddle with the comments to reflect this.


================
Comment at: llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup-2.mir:40
+    $ebx = LEA32r killed $ebp, 1, $ebp, 0, $noreg, debug-instr-number 1
+    ; jmorse, i386 -mcpu=corei7-avx
+    RETQ $ebx
----------------
Orlando wrote:
> What is this comment for?
Reminding myself what flags were required to stimulate the substitution -- hence this file targets i386, but tunes the cpu to corei7. I'll delete this when committing.


================
Comment at: llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup.mir:5
+#
+# Test each call to substitute debug instr-ref labeles in the fixup-leas pass.
+# Some are only reachable under specific CPU modes it seems -- each function
----------------
Orlando wrote:
> I'm finding it difficult to understand this opening sentence and nit: labeles -> labels 
> 
> 
Rewriting when landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105820