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: Update IL offsets in fgExpandStaticInitForCall #99662

Merged
merged 4 commits into from Mar 21, 2024
Merged

Conversation

TIHan
Copy link
Member

@TIHan TIHan commented Mar 13, 2024

Will resolve #99543

See comment for more details.

@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 Mar 13, 2024
Copy link
Contributor

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


IL_OFFSET splitPointILOffset = fgFindBlockILOffset(newBlock);
IL_OFFSET splitPointILOffset = fgFindBlockILOffset(newBlock);
Copy link
Member Author

@TIHan TIHan Mar 13, 2024

Choose a reason for hiding this comment

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

Instead of doing the if (!newBlock->HasFlag(BBF_INTERNAL)) check, I was debating whether or not fgFindBlockILOffset should always return BAD_IL_OFFSET because it currently will not.

Copy link
Member

Choose a reason for hiding this comment

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

Is this the place that caused trouble? Because the before logic seemed ok.

@TIHan TIHan marked this pull request as ready for review March 15, 2024 00:44
@TIHan
Copy link
Member Author

TIHan commented Mar 15, 2024

// cc @dotnet/jit-contrib @AndyAyersMS @jakobbotsch

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.

Still not sure about this... left some notes over in #99543


IL_OFFSET splitPointILOffset = fgFindBlockILOffset(newBlock);
IL_OFFSET splitPointILOffset = fgFindBlockILOffset(newBlock);
Copy link
Member

Choose a reason for hiding this comment

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

Is this the place that caused trouble? Because the before logic seemed ok.

@@ -6124,10 +6127,10 @@ BasicBlock* Compiler::fgNewBBFromTreeAfter(
{
BasicBlock* newBlock = fgNewBBafter(jumpKind, block, true);
newBlock->SetFlags(BBF_INTERNAL);
assert(newBlock->bbCodeOffs == BAD_IL_OFFSET);
assert(newBlock->bbCodeOffsEnd == BAD_IL_OFFSET);
Copy link
Member

Choose a reason for hiding this comment

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

I would actually give this the same logic as above, use fgFindBlockILOffset to figure out the proper offsets, if any.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we do that for BBF_INTERNAL blocks?

@TIHan
Copy link
Member Author

TIHan commented Mar 15, 2024

Is this the place that caused trouble? Because the before logic seemed ok.

Yes. If fgFindBlockILOffset returned BAD_IL_OFFSET it would also fix the issue.

@TIHan TIHan changed the title JIT: Do not set the code offsets for BBF_INTERNAL blocks JIT: Setting block->bbCodeOffsEnd to BAD_IL_OFFSET when expanding static init for calls Mar 19, 2024
@TIHan TIHan changed the title JIT: Setting block->bbCodeOffsEnd to BAD_IL_OFFSET when expanding static init for calls JIT: Do not update IL offsets in fgSplitBlockAfterStatement if the original block's ending offset was BAD_IL_OFFSET Mar 20, 2024
@TIHan TIHan changed the title JIT: Do not update IL offsets in fgSplitBlockAfterStatement if the original block's ending offset was BAD_IL_OFFSET JIT: Update IL offsets in fgExpandStaticInitForCall Mar 20, 2024
@TIHan TIHan merged commit 090e349 into dotnet:main Mar 21, 2024
108 of 110 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 21, 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.

Assertion failed 'lastBlockILEndOffset < beginOffs'
2 participants