Page MenuHomePhabricator

RegAllocFast: Record internal state based on register units
ClosedPublic

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

Details

Summary

Record internal state based on register units. This is often more
efficient as there is typically less register units to update compared
to iterating over all the aliases of a register.

Diff Detail

Event Timeline

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

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

arsenm accepted this revision.Nov 9 2018, 4:48 PM

LGTM

This revision is now accepted and ready to land.Nov 9 2018, 4:48 PM

On its own this introduces assertions in some inline asm with tied operands testcases:
Assertion failed: (RegUnitStates[*UI] == VirtReg && "inverse map valid"), function dumpState, file ../lib/CodeGen/RegAllocFast.cpp,

This particular one no longer appears with all of the patches, although it seems there are some others.

Herald added a project: Restricted Project. · View Herald TranscriptMar 19 2019, 4:25 PM
Herald added a subscriber: jdoerfert. · View Herald Transcript
arsenm commandeered this revision.Jul 31 2019, 6:40 PM
arsenm edited reviewers, added: MatzeB; removed: arsenm.
This revision now requires review to proceed.Jul 31 2019, 6:40 PM
arsenm updated this revision to Diff 212711.Jul 31 2019, 6:42 PM

Tests now all pass, except CodeGen/X86/2010-06-28-FastAllocTiedOperand.ll

This ends up allocating a physical register already implicitly def'd to a tied vreg operand. The subsequent patch fixes it, so I'm not sure it's worth much effort trying to extract the relevant parts from the other patch

qcolombet accepted this revision.Sep 4 2019, 12:28 PM

LGTM.

Suggestion of a small refactoring below.

lib/CodeGen/RegAllocFast.cpp
434 ↗(On Diff #212711)

Maybe we can factor this code out into a verify kind of thing.
Like one that returns a bool that we wrap into an assert here and in the killVirtReg.

This revision is now accepted and ready to land.Sep 4 2019, 12:28 PM
arsenm updated this revision to Diff 268294.Jun 3 2020, 1:45 PM

Rebase and fix the one remaining failure in test/CodeGen/X86/2010-06-28-FastAllocTiedOperand.ll, so now this patch no longer introduces an intermediate broken step before D52010.

The problem was the spilling was dropped from handleThroughOperands in the physdef collision loop. Restored the definePhysReg call with the new regunit iterator checks instead of using the reg alias iterator

arsenm closed this revision.Jun 3 2020, 1:57 PM

Posted final revision to help post-commit review. The previous revision was accepted, and in order to escape the 2 year deadlock on this set of patches, I've committed this as 66251f7e1de79a7c1620659b7f58352b8c8e892e

uabelho added inline comments.
llvm/lib/CodeGen/RegAllocFast.cpp
136

This seems to be unused? gcc warns with

../lib/CodeGen/RegAllocFast.cpp:245:6: warning: 'bool {anonymous}::RegAllocFast::isPhysRegFree(llvm::MCPhysReg) const' defined but not used [-Wunused-function]
bool RegAllocFast::isPhysRegFree(MCPhysReg PhysReg) const {

^~~~~~~~~~~~

Will it be used soon or can it be removed?

arsenm marked an inline comment as done.Jun 4 2020, 6:51 AM
arsenm added inline comments.
llvm/lib/CodeGen/RegAllocFast.cpp
136

Removed