Skip to content

Conversation

@amanasifkhalid
Copy link
Contributor

Follow-up to #99362. Adds helper methods for redirecting BBJ_COND blocks' true/false targets to new successors while reusing the existing flow edge.

cc @dotnet/jit-contrib, @AndyAyersMS PTAL. This may conflict with #99541; I'm ok with waiting for that to be merged first.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 11, 2024
@amanasifkhalid
Copy link
Contributor Author

Diffs are quite big, though the bulk of them seem to come from a few collections. I'll dig into these locally.

@amanasifkhalid
Copy link
Contributor Author

Diffs are more reasonable now that #99541 has been checked in. Looking at them locally, edge weights diffs are driving churn in fgReorderBlocks.

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

Code LGTM. Left some notes on comments.

newTarget->bbRefs++;
}

void Compiler::fgRedirectTrueEdge(BasicBlock* block, BasicBlock* newTarget)
Copy link
Member

Choose a reason for hiding this comment

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

Please add a header comment block for this method

assert(newTarget->checkPredListOrder());
}

void Compiler::fgRedirectFalseEdge(BasicBlock* block, BasicBlock* newTarget)
Copy link
Member

Choose a reason for hiding this comment

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

Please add a header comment block for this method


newTarget->bbRefs++;

// Pred list of target should (stilL) be ordered
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
// Pred list of target should (stilL) be ordered
// Pred list of target should (still) be ordered


newTarget->bbRefs++;

// Pred list of target should (stilL) be ordered
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
// Pred list of target should (stilL) be ordered
// Pred list of target should (still) be ordered

@amanasifkhalid
Copy link
Contributor Author

@AndyAyersMS thanks for the review -- I incorporated your feedback.

@amanasifkhalid amanasifkhalid merged commit 7878f4e into dotnet:main Mar 12, 2024
@amanasifkhalid amanasifkhalid deleted the update-succ-edge branch March 12, 2024 22:34
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
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