This is an archive of the discontinued LLVM Phabricator instance.

RegAllocGreedy: Account for reserved registers in num regs heuristic
ClosedPublic

Authored by arsenm on Aug 23 2021, 1:10 PM.

Details

Summary

This simple heuristic uses the estimated live range length combined
with the number of registers in the class to switch which heuristic to
use. This was taking the raw number of registers in the class, even
though not all of them may be available. AMDGPU heavily relies on
dynamically reserved numbers of registers based on user attributes to
satisfy occupancy constraints, so the raw number is highly misleading.

There are still a few problems here. In the original testcase that
made me notice this, the live range size is incorrect after the
scheduler rearranges instructions, since the instructions don't have
the original InstrDist offsets. Additionally, I think it would be more
appropriate to use the number of disjointly allocatable registers in
the class. For the AMDGPU register tuples, there are a large number of
registers in each tuple class, but only a small fraction can actually
be allocated at the same time since they all overlap with each
other. It seems we do not have a query that corresponds to the number
of independently allocatable registers. Relatedly, I'm still debugging
some allocation failures where overlapping tuples seem to not be
handled correctly.

The test changes are mostly noise. There are a handful of x86 tests
that look like regressions with an additional spill, and a handful
that now avoid a spill. The worst looking regression is likely
test/Thumb2/mve-vld4.ll which introduces a few additional
spills. test/CodeGen/AMDGPU/soft-clause-exceeds-register-budget.ll
shows a massive improvement by completely eliminating a large number
of spills inside a loop.

Diff Detail

Event Timeline

arsenm created this revision.Aug 23 2021, 1:10 PM
arsenm requested review of this revision.Aug 23 2021, 1:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 23 2021, 1:10 PM
Herald added subscribers: MaskRay, wdng. · View Herald Transcript

The change itself looks reasonable to me.

The worst looking regression is likely test/Thumb2/mve-vld4.ll

We likely get the register pressure very wrong, and it looks like less code in total. I have no objections to this patch from an Arm point of view.

Arm MVE has 32 scalar FP S regs, 4 (aligned) of which make up the 8 vector Q regs (so q0=s0-s1-s2-s3, q1=s4-s5-s6-s7 etc).
4 potentially unaligned Q registers make up a QQQQPR reg (q0-q1-q2-q3, q1-q2-q3-q4, q2-q3-q4-q5, q3-q4-q5-q6 and q4-q5-q6-q7)
So we have "5" QQQQPR registers, only 2 of which are actually allocatable at once.

Additionally, I think it would be more appropriate to use the number of disjointly allocatable registers in the class. For the AMDGPU register tuples, there are a large number of registers in each tuple class, but only a small fraction can actually be allocated at the same time since they all overlap with each other. It seems we do not have a query that corresponds to the number of independently allocatable registers.

That sounds very useful.

lkail added a subscriber: lkail.Sep 13 2021, 7:08 AM
qcolombet accepted this revision.Sep 14 2021, 1:28 PM
This revision is now accepted and ready to land.Sep 14 2021, 1:28 PM
paklui added a subscriber: paklui.Sep 15 2021, 11:56 AM