This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIT: add some devirtualization info to the inline context #20395
JIT: add some devirtualization info to the inline context #20395
Changes from all commits
c442ea1
8f381fb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While you are here, could you please fix headers/signature for
InlineContext* InlineStrategy::NewSuccess(InlineInfo* inlineInfo)
and
InlineContext* InlineStrategy::NewFailure(GenTreeStmt* stmt, InlineResult* inlineResult)
,they probably should be the same (and the headers are), but
NewSuccess
doesn't havestmt - statement containing call being inlined
as argument.Then maybe it will be obvious how to extract this two similar initialization lists into one method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll update the comment, but are you suggesting common out some of the work these two methods do?
It may be clunky as they don't have access to the same data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can delete
GenTreeStmt* stmt
arg fromNewFailure
and do whatNewSuccess
does:GenTreeStmt* stmt = inlineInfo->iciStmt;
calleeContext->m_Offset = stmt->AsStmt()->gtStmtILoffsx;
<- looks like we do not need this cast here, it is already a statement.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently don't have access to the
InlineInfo
for the failure cases.I'll remove the cast too.