Skip to content

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.

@ghost ghost 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