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: Follow related intervals for single-reg LIR temp intervals #86632

Merged

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented May 23, 2023

Not sure why the code stops following related intervals for LIR temp intervals preferenced to a single register... this removes that restriction.

I hit some regressions in #86388 due to this difference between intervals for locals and intervals for LIR temps.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 23, 2023
@ghost ghost assigned jakobbotsch May 23, 2023
@ghost
Copy link

ghost commented May 23, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Not sure why the code stops following related intervals for LIR temp intervals preferenced to a single register, but maybe the diffs will tell me.

I hit some regressions in #86388 due to this difference between intervals for locals and intervals for LIR temps.

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch jakobbotsch force-pushed the lsra-preference-intervals-lir-temps branch from 7153700 to 6347ba6 Compare May 23, 2023 11:05
@jakobbotsch jakobbotsch marked this pull request as ready for review May 23, 2023 17:53
@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @kunalspathak

Small number of mostly positive diffs. Sometimes this introduces a new spill; I checked, and that happens because we do not preference LIR edges crossing calls to callee-saved registers. I can try to reopen #81242 after to deal with that.

@jakobbotsch jakobbotsch requested a review from kunalspathak May 23, 2023 17:56
@kunalspathak
Copy link
Member

related intervals for LIR temp intervals

How do we know that this only affects for LIR temp intervals?

@jakobbotsch
Copy link
Member Author

jakobbotsch commented May 23, 2023

How do we know that this only affects for LIR temp intervals?

All the cases I looked at were LIR temp intervals. Are you worried about some other type of intervals? This loop is going through related intervals and only looking for definitions in those intervals, so I'm not sure it would look at any other kinds of intervals (but in any case, shouldn't the preferencing here work the same regardless?)

@kunalspathak
Copy link
Member

Are you worried about some other type of intervals

Exactly. We also create relatedIntervals for other cases like vector save/restore. I am not entirely sure of why we had isLocalVar check to begin with, and so wanted to see if the only come from LIR temp intervals. Regardless, I think this is safe to do.

@jakobbotsch jakobbotsch merged commit c368b3d into dotnet:main May 24, 2023
@jakobbotsch jakobbotsch deleted the lsra-preference-intervals-lir-temps branch May 24, 2023 09:31
@ghost ghost locked as resolved and limited conversation to collaborators Jun 23, 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 this pull request may close these issues.

2 participants