[PATCH] D87744: [RegisterCoalescer] passs Undefs to extendToIndices()

[Ruiling, Song via Phabricator via llvm-commits]

ruiling added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/coalescer-removepartial-extend-undef-subrange.mir:20
+# chain and assert:
+# "Use of $noreg does not have a corresponding definition on every path
+#  LLVM ERROR: Use not jointly dominated by defs"
----------------
arsenm wrote:
> $noreg doesn't have liveness?
I am not sure what you are really asking here. I know the assert message is a little bit misleading. The reason it output $noreg is we are passing 0 to argument 'PhyReg' of LiveRangeCalc::extend() in LiveIntervals::extendToIndices(). When assert happens, we are trying to extend a subrange of register %90. The offending subrange has a def point in one of bb.4 predecessor chain, so it has liveness in this path. but in another predecessor chain 4->27->1->0, it only has one undef point in bb.0.


================
Comment at: llvm/test/CodeGen/AMDGPU/coalescer-removepartial-extend-undef-subrange.mir:29
+  bb.0:
+    successors: %bb.1, %bb.2
+    liveins: $sgpr2, $sgpr3, $vgpr3
----------------
arsenm wrote:
> Can you reduce this any further? -run-pass=none will also compact the register numbers
I just tried to remove each one of the most blocks containing instructions in MIR, I failed to trigger the crash after removing them. If you have any good idea to simplify further, I can try it. Some more notes: The test case is already a much simplified version than original failure IR with 100+ basic blocks. I have tried removing each basic block in IR before and it would cause problem disappear. The problem is hard to trigger, it also depends on the order of coalescing work. For example, if I reorder the blocks, the problem may disappear because the coalescing of registers will be different then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87744