Page MenuHomePhabricator

RegAllocFast: Add heuristic to detect values not live-out of a block
ClosedPublic

Authored by arsenm on Nov 9 2018, 4:33 PM.

Details

Summary

Add an improved/new heuristic to catch more cases when values are not live out of a basic block.

Diff Detail

Event Timeline

MatzeB created this revision.Nov 9 2018, 4:33 PM
MatzeB added a comment.Nov 9 2018, 4:33 PM

This is part of my rewrite regallocfast series. See also D52010

I see regressions with this one after some possibly recent commits? In the first test case in test/CodeGen/AMDGPU/basic-branch.ll, it seems to not detect the use of the pointer value and omits the spill. The restore is still present

arsenm added inline comments.Nov 26 2018, 12:03 PM
lib/CodeGen/RegAllocFast.cpp
272

I think this approach doesn't work because once the virtual register is rewritten in one block, it no longer appears in the vreg's use list in the next block

arsenm added inline comments.Mar 15 2019, 4:53 PM
lib/CodeGen/RegAllocFast.cpp
106

Typo, extra c in Across

276–277

This is broken because it's skipping the first use. It can also useMRI->reg_nodbg_instructions.

This is also broken for the case where there is a phi/copy when there is a loop back to the same block. This needs to check for whether the use instruction happens before the spilled value def

Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2019, 4:53 PM
arsenm commandeered this revision.Mar 18 2019, 4:19 PM
arsenm updated this revision to Diff 191203.
arsenm edited reviewers, added: MatzeB; removed: arsenm.

Fix use scan skipping first use. Assume live-out if block is its own successor.

Update tests. Not sure what's going on with test/DebugInfo/X86/pieces-1.ll, but I doubt it's testing what it's supposed to

qcolombet accepted this revision.Fri, May 3, 8:50 AM

LGTM. Nitpicks below.

lib/CodeGen/RegAllocFast.cpp
106

The typo is in the comment is still here :).

282

Just a thought, given that we already traversed all the instructions of MBB at this point, we could avoid this loop all together by checking if we saw as many uses as the total number of uses for each virtual register.
That would be valuable only if checking the total number of the uses is O(1), which I am not sure it is right now. Basically, although we have a limit of 8 in that patch, I am worried this loop might have an impact on compile time.

Bonus, if we were to do that, we would know what the status for the back-edges case.

Anyhow, nothing to do here!

1057

For clarity
/*OnlyLiveOut=*/false

1167

Ditto

This revision is now accepted and ready to land.Fri, May 3, 8:50 AM
arsenm closed this revision.Fri, May 3, 10:02 AM
arsenm marked 2 inline comments as done.

r359906