[PATCH] D141576: [CodeGen] Prevent overlapping subregs in getCoveringSubRegIndexes

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh marked 4 inline comments as done.
Pierre-vh added a comment.

In D141576#4051805 <https://reviews.llvm.org/D141576#4051805>, @arsenm wrote:

> In D141576#4046355 <https://reviews.llvm.org/D141576#4046355>, @Pierre-vh wrote:
>
>> Testing for this is a bit tricky, fortunately it looks like it's already covered by a test.
>> If another form of testing is needed, perhaps we could do a unit test? I feel like it'd be a lot more straightforward to just query TRI in a unit test and check all the corner cases there.
>
> You already have the reduced mir test?

Only for the virtregrewriter pass (so with already split liveranges). Plus, the testcase I have seems highly sensitive to register allocation changes, and as liverange splitting happens in the register allocator it's not a good test case as it may start passing not because of this change but because of other unrelated changes that affect regalloc (for instance it just passes now on trunk without any changes)
Any test case could also be made irrelevant if we add the missing 13-15 tuples subregister indexes

Should I try to come up with an artificial case like the ones in splitkit.mir? What should the structure be to tickle the register allocator just enough to make liverange splitting copy sub0-sub29 of a sreg_1024?



================
Comment at: llvm/test/CodeGen/AMDGPU/extend-phi-subrange-not-in-parent.mir:29
   ; CHECK-NEXT: {{  $}}
-  ; CHECK-NEXT:   dead %3:vreg_64 = IMPLICIT_DEF
+  ; CHECK-NEXT:   dead [[DEF2:%[0-9]+]]:vreg_64 = IMPLICIT_DEF
   ; CHECK-NEXT:   S_NOP 0, implicit [[DEF1]]
----------------
foad wrote:
> Did anything actually change here? If not can you first commit a patch to regenerate the checks for this file, and then rebase this patch?
That indeed didn't change in this patch, I regenerated the test
https://github.com/llvm/llvm-project/commit/9bdfd7c8db2066e5b50b9599248660bdf8eda7f0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141576