[PATCH] D29048: [RegisterCoalescer] Do not call LiveIntervals::getInstructionIndex with a DBG_VALUE

[Matthias Braun via Phabricator via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

Thanks for providing the testcase, that made investigating this easier. The fix here seems reasonable to me as well.

However I was surprised to see this problem popping up at all, so I dug a bit deeper:

- This problem only occurs for `DBG_VALUE undef %X` which doesn't make much sense (but is legal)
- The undef flag there is actually produced by code in the same function a bit further down (`// A subreg use of a partially undef...`) which right now looks like a dangerous bug to me (in that it adds an undef flag here even though the register seems live to me). I will investigate this further.

I'm fine with the patch going in, if the testcase is cleaned up (see below):



================
Comment at: test/CodeGen/AMDGPU/regcoalesce-dbg.mir:1
+# RUN: llc -march=amdgcn -run-pass simple-register-coalescing -o - %s
+# REQUIRES: asserts
----------------
Good tests should check some output.
Or put another way: The test should fail if someone replaces llc with /usr/bin/true.


================
Comment at: test/CodeGen/AMDGPU/regcoalesce-dbg.mir:7
+--- |
+  ; ModuleID = '<stdin>'
+  source_filename = "<stdin>"
----------------
You should be able to remove the complete IR part from the test and replace it with a dummy `define void @test() { ret void }`. The only thing you need to do is remove IR back-reference in the MIR part. Which for this case should just be removing the `(%ir-block.0)` from the bb.0 label.


================
Comment at: test/CodeGen/AMDGPU/regcoalesce-dbg.mir:75-80
+alignment:       0
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
----------------
you should be able to remove alignemnt, exposesReturnsTwice, legalize, regBankSelected, selected.


================
Comment at: test/CodeGen/AMDGPU/regcoalesce-dbg.mir:106-119
+frameInfo:       
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
----------------
You can probably remove the frameInfo completely with the test still working.


https://reviews.llvm.org/D29048