This is an archive of the discontinued LLVM Phabricator instance.

RegAllocGreedy: Fix detection of lanes read by a bundle
ClosedPublic

Authored by arsenm on Mar 24 2023, 6:40 PM.

Details

Summary

SplitKit creates questionably formed bundles of copies
when it needs to copy a subset of live lanes and can't do
it with a single subregister index. These are merely marked
as part of a bundle, and don't start with a BUNDLE instruction.
Queries for the slot index would give the first copy in the
bundle, and we need to inspect the operands of all the other
bundled copies.

Also fix and simplify detection of read lane subsets. This causes
some RISCV test regressions, but these look like accidentally beneficial
splits. I don't see a subrange based reason to perform these splits.

Avoids some really ugly regressions in a future patch.

Diff Detail

Event Timeline

arsenm created this revision.Mar 24 2023, 6:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2023, 6:40 PM
arsenm requested review of this revision.Mar 24 2023, 6:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2023, 6:40 PM
qcolombet added inline comments.Mar 29 2023, 2:20 AM
llvm/lib/CodeGen/RegAllocGreedy.cpp
1262

Out-of-curiosity why does AnalyzeVirtRegInBundle requires non-const instructions?

1272

Given the added isUndef here, I think we want to continue for isUndef after (or before) that if.
I.e., I think we want to skip line 1279.

1310

I believe the previous version was correct.

arsenm updated this revision to Diff 556645.Sep 13 2023, 3:16 AM
arsenm marked an inline comment as done.
arsenm added inline comments.Sep 13 2023, 3:16 AM
llvm/lib/CodeGen/RegAllocGreedy.cpp
1262

It shouldn't. The one possible reason would be if you want a non-const pointer in the output array

arsenm updated this revision to Diff 556646.Sep 13 2023, 3:17 AM
qcolombet accepted this revision.Sep 18 2023, 5:54 AM
This revision is now accepted and ready to land.Sep 18 2023, 5:54 AM