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

Jit: move runtime lookup expansion to lower #35551

Closed
erozenfeld opened this issue Apr 28, 2020 · 1 comment · Fixed by #81635
Closed

Jit: move runtime lookup expansion to lower #35551

erozenfeld opened this issue Apr 28, 2020 · 1 comment · Fixed by #81635
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@erozenfeld
Copy link
Member

erozenfeld commented Apr 28, 2020

This is a follow-up to #341.

The jit currently expands runtime lookups in the importer (impRuntimeLookupToTree). The optimal expansion requires introducing control flow either via GT_QMARK or via IndirectCallTransformer.

The tail call via helpers mechanism introduced in #341 also needs to insert runtime lookups. The expansion of tail calls via helpers happens in morph and it's impossible to change control flow there the way it's done in the importer. Because of that the runtime lookups introduced in morph use the slower helper call expansion:

// If the first condition is true, runtime lookup tree is available only via the run-time helper function.
// TODO-CQ If the second or third condition is true, we are always using the slow path since we can't
// introduce control flow at this point. See impRuntimeLookupToTree for the logic to avoid calling the helper.
// The long-term solution is to introduce a new node representing a runtime lookup, create instances
// of that node both in the importer and here, and expand the node in lower (introducing control flow if
// necessary).
return gtNewRuntimeLookupHelperCallNode(pRuntimeLookup,
getRuntimeContextTree(pLookup->lookupKind.runtimeLookupKind),
compileTimeHandle);

The suggested solution is to move the expansion of runtime lookups to lower so that both the importer and morph can just create and insert a new node representing a runtime lookup and have lower expand it in an optimal way. This will involve teaching optimization phases (e.g., value numbering) about the new runtime lookup node.

category:cq
theme:helpers
skill-level:intermediate
cost:medium

@erozenfeld erozenfeld added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 28, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Apr 28, 2020
@AndyAyersMS AndyAyersMS removed the untriaged New issue has not been triaged by the area owner label May 6, 2020
@AndyAyersMS AndyAyersMS added this to the Future milestone May 6, 2020
@AndyAyersMS
Copy link
Member

@erozenfeld marking this as future; let me know if you think we should try to implement this in 5.0.

@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@BruceForstall BruceForstall removed the JitUntriaged CLR JIT issues needing additional triage label Nov 14, 2020
@EgorBo EgorBo self-assigned this Feb 2, 2023
@EgorBo EgorBo modified the milestones: Future, 8.0.0 Feb 2, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 4, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 14, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 14, 2023
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 a pull request may close this issue.

5 participants