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

proof of concept #90058

Closed
wants to merge 4 commits into from
Closed

proof of concept #90058

wants to merge 4 commits into from

Conversation

AndyAyersMS
Copy link
Member

FYI @EgorBo @jakobbotsch

This gets the ?: case Egor was discussing and maybe the local address one Jakob was looking at.

Not sure this is the best approach, but it does some interesting things. It leverage the local threading so hopefully isn't' too costly.

@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 Aug 4, 2023
@ghost ghost assigned AndyAyersMS Aug 4, 2023
@ghost
Copy link

ghost commented Aug 4, 2023

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

Issue Details

FYI @EgorBo @jakobbotsch

This gets the ?: case Egor was discussing and maybe the local address one Jakob was looking at.

Not sure this is the best approach, but it does some interesting things. It leverage the local threading so hopefully isn't' too costly.

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS
Copy link
Member Author

Sample diff

image

@jakobbotsch
Copy link
Member

maybe the local address one Jakob was looking at

To handle these cases we need to do something before or during local morph, otherwise we will still end up with address exposure.

@AndyAyersMS
Copy link
Member Author

This is like an anti-head-merge... if we were to take this along with head merge we'd need to figure out how not to undo what we just did.


bool Compiler::fgTailDuplicateBlock(BasicBlock* const block)
{
// We only consider blocks with muliple preds without
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// We only consider blocks with muliple preds without
// We only consider blocks with multiple preds without

@jakobbotsch
Copy link
Member

jakobbotsch commented Aug 17, 2023

This is like an anti-head-merge

Isn't this more like an anti-tail-merge? It duplicates the same statement at the end of multiple predecessors, so those statements are candidates for tail merging.

@AndyAyersMS
Copy link
Member Author

This is like an anti-head-merge

Isn't this more like an anti-tail-merge? It duplicates the same statement at the end of multiple predecessors, so those statements are candidates for tail merging.

Yes, you're right. Actually an anti-head-merge (head duplication? I guess) might be interesting to consider, if we think the successor blocks have more interesting context than the current block.

@ghost ghost closed this Sep 16, 2023
@ghost
Copy link

ghost commented Sep 16, 2023

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 16, 2023
This pull request was closed.
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.

3 participants