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

Mon Jan 16 10:34:02 PST 2023

arsenm added a comment.

In D141576#4055520 <https://reviews.llvm.org/D141576#4055520>, @Pierre-vh wrote:

> 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)

This is a general issue, still having something is better than nothing

> Any test case could also be made irrelevant if we add the missing 13-15 tuples subregister indexes

Could also try forcing it on a different target

> 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?

That would also be good, it can be tricky and time consuming but it’s usually possible. It’s a bit of an art to craft these tests

> Maybe I can do something based on extend-phi-subrange-not-in-parent, just cleaned up and with a comment saying exactly what it needs to test.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141576