This is an archive of the discontinued LLVM Phabricator instance.

[WIP] Bias statepoint operands towards spilling
AbandonedPublic

Authored by reames on Feb 18 2021, 10:33 AM.

Details

Reviewers
bollu
Summary

Not for review, simply saving an idea for later.

The idea behind this is the a statepoint operand which can be spilled should contribute to the spill cost of a live interval as it's equally happy to use register and stack.

This made a difference on a larger test I'd looked at, but I haven't managed to reduce a small test case. All of the small test cases I write have obviously bigger problems (quality wise).

Setting this aside until other issues resolved.

Diff Detail

Event Timeline

reames created this revision.Feb 18 2021, 10:33 AM
reames requested review of this revision.Feb 18 2021, 10:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 18 2021, 10:33 AM
reames abandoned this revision.Feb 18 2021, 10:34 AM

The code looks good.

llvm/lib/CodeGen/CalcSpillWeights.cpp
136

Not the first case I see this switch, may be it makes sense to extract to utility function.

157

consider using findRegisterUseOperandIdx and findRegisterDefOperandIdx but might be it is an overhead.

289

You could skip all thing above as you want weight == 0 anyway.

dantrushin added inline comments.Feb 20 2021, 1:43 AM
llvm/lib/CodeGen/CalcSpillWeights.cpp
157

isn't this loop is equivalent to

for i in [NumDefs, StartIdx) {
  if MO[i].Reg == Reg
    return false;
}
return true;

?

Hi Philip, I've landed the test CodeGen/X86/statepoint-ra-no-ls.ll which shows the immediate impact of this patch.
It shows that with this patch RA prefer to evict the VR with use in statepoint var use and as a result we eliminate redundant spill.

However the test is very limited. The reason of this is a split. So the test is written to avoid any splits.
Note that spill weight depends on the weight of spill/fill operation but also on size of live interval which is used in
normalization. This effect is very important on small intevals.

If you add additional statepoint instruction into the test Local Split will be triggered and it breaks everything. The same problem will be with region split, but let's start first with local split.
First of all Local Split (RAGreedy::tryLocalSplit) also estimates the spill weight of the future split live interval and does not take into account statepoint var operands (see computation of EstWeight).
As a result its computed value will be bigger then VirtRegAuxInfo::weightCalcHelper produces. It should be fixed actually to be consistent.
However it is not enough. Split operation introduce additional use in COPY at end of interval. As a result spill weight will be at least 2 before normalization.
On small intervals where number of def/uses it is big enough and size of live interval starts playing significant role.
I though about prohibit splitting on statepoint instruction but not sure it is really profitable in all cases. For example if we evict another VR, probably it might be split in a way that all intervals will be on physical registers and no need in spilling at all.
In other words I want to prefer spill of VR with use in statepoint but only if we really need to spill anyone.
Does it make sense? Any thought?