Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combine free and busy register allocation #45135

Merged
merged 17 commits into from
Dec 7, 2020

Conversation

CarolEidt
Copy link
Contributor

@CarolEidt CarolEidt commented Nov 24, 2020

Combine register selection to consider free and busy registers in the same method.
Refactor the selection to make it easier to reorder the criteria.

Edit by @kunalspathak:

Fixes: #9399

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Nov 24, 2020
@CarolEidt
Copy link
Contributor Author

@dotnet/jit-contrib PTAL

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did the first pass and added some questions/clarifications and minor suggestions.

src/coreclr/src/jit/lsra.h Show resolved Hide resolved
src/coreclr/src/jit/lsra.h Show resolved Hide resolved
src/coreclr/src/jit/lsra.h Outdated Show resolved Hide resolved
@@ -10431,7 +10620,7 @@ void LinearScan::dumpRegRecords()
#endif // FEATURE_PARTIAL_SIMD_CALLEE_SAVE
printf("%c", activeChar);
}
else if (regRecord.isBusyUntilNextKill)
else if ((genRegMask(regNum) & regsBusyUntilKill) != RBM_NONE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use isRegBusy()?

src/coreclr/src/jit/lsra.cpp Show resolved Hide resolved
src/coreclr/src/jit/lsra.cpp Show resolved Hide resolved
src/coreclr/src/jit/lsra.cpp Outdated Show resolved Hide resolved
src/coreclr/src/jit/lsra.cpp Outdated Show resolved Hide resolved
src/coreclr/src/jit/lsra.cpp Outdated Show resolved Hide resolved

doubleReg = reg->regNum;
assert(genIsValidDoubleReg(doubleReg));
RegRecord* anotherHalfReg = getSecondHalfRegRec(reg);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering why don't we do something like else below i.e. call findAnotherHalfRegRec() instead of getSecondHalfRegRec()? Do things vary if assignedInterval's registerType == TYP_DOUBLE ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment of this method always expect reg to be lower half: "reg" should be a even-numbered float register, i.e. lower half of double register.

Copy link
Contributor Author

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks!

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kunalspathak kunalspathak merged commit 86bdbf7 into dotnet:master Dec 7, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[LSRA][RyuJIT] Consider merging allocating free & busy regs
5 participants