Skip to content

Replace CORCOMPILE_FIXUP_BLOB_KIND with ReadyToRunFixupKind #115854

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

Merged
merged 14 commits into from
May 27, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented May 22, 2025

Removes the redundant CORCOMPILE_FIXUP_BLOB_KIND enum and replaces all its uses with the equivalent ReadyToRunFixupKind enum. CORCOMPILE_FIXUP_BLOB_KIND was a left-over from fragile NGen.

Fixes #115853.

@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label May 22, 2025
@jkotas jkotas added area-ReadyToRun-coreclr and removed linkable-framework Issues associated with delivering a linker friendly framework labels May 22, 2025
@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label May 22, 2025
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] CORCOMPILE_FIXUP_BLOB_KIND is redundant code Replace CORCOMPILE_FIXUP_BLOB_KIND with ReadyToRunFixupKind May 22, 2025
@Copilot Copilot AI requested a review from jkotas May 22, 2025 01:43
Copilot finished work on behalf of jkotas May 22, 2025 01:43
@jkotas
Copy link
Member

jkotas commented May 22, 2025

@copilot You have missed replacing the usage of this enum in LoadDynamicInfoEntry method. Could you please fix that?

…de in LoadDynamicInfoEntry

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented May 22, 2025

@copilot You have missed replacing the usage of this enum in LoadDynamicInfoEntry method. Could you please fix that?

I've fixed the missed replacement in the LoadDynamicInfoEntry method. The changes replace ENCODE_MODULE_OVERRIDE with ReadyToRunFixupKind::ModuleOverride in the implementation. See commit 02688ff.

Copilot finished work on behalf of jkotas May 22, 2025 02:00
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot finished work on behalf of jkotas May 22, 2025 02:21
@platform-cane891

This comment was marked as spam.

@jkotas jkotas removed the linkable-framework Issues associated with delivering a linker friendly framework label May 22, 2025
@jkotas jkotas marked this pull request as ready for review May 22, 2025 03:26
@jkotas jkotas requested a review from AaronRobinsonMSFT May 22, 2025 03:28
Comment on lines 3161 to 3163
// case READYTORUN_FIXUP_VirtualEntry_DefToken:
// case READYTORUN_FIXUP_VirtualEntry_RefToken:
// case READYTORUN_FIXUP_VirtualEntry_Slot:
Copy link
Member

Choose a reason for hiding this comment

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

Should these be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the commented out case statements as suggested. See commit daf6dae.

@@ -3107,11 +3107,11 @@ PCODE DynamicHelperFixup(TransitionBlock * pTransitionBlock, TADDR * pCell, DWOR
BYTE kind = *pBlob++;
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to just cast/type this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the code to cast kind to ReadyToRunFixupKind and also applied appropriate casting in the second line where we clear the ModuleOverride flag. This makes the type usage consistent with the enum we're now using. See commit daf6dae.

Co-authored-by: AaronRobinsonMSFT <30635565+AaronRobinsonMSFT@users.noreply.github.com>
Copilot finished work on behalf of AaronRobinsonMSFT May 22, 2025 04:15
}

switch ((CORCOMPILE_FIXUP_BLOB_KIND) signatureKind)
switch ((ReadyToRunFixupKind) signatureKind)
Copy link
Member

Choose a reason for hiding this comment

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

@copilot Move (ReadyToRunFixupKind) cast to signatureKind declaration like in the other change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved the ReadyToRunFixupKind cast to the signatureKind variable declaration as requested and also added appropriate casting when clearing the ModuleOverride flag. See commit 539b621.

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot finished work on behalf of jkotas May 22, 2025 04:21
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot finished work on behalf of jkotas May 22, 2025 04:27
Copilot finished work on behalf of jkotas May 22, 2025 04:36
Copilot finished work on behalf of jkotas May 22, 2025 04:42
vietnguyengit

This comment was marked as spam.

Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT left a comment

Choose a reason for hiding this comment

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

@jkotas I don't see anything here obviously wrong. Most of it looks like simple pattern replacement.

@AaronRobinsonMSFT
Copy link
Member

Do we want to run the outerloop or crossgen2 legs?

@jkotas
Copy link
Member

jkotas commented May 27, 2025

/azp run runtime-coreclr crossgen2 outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jkotas
Copy link
Member

jkotas commented May 27, 2025

/ba-g known android timeout

@jkotas jkotas merged commit b10ea03 into main May 27, 2025
121 of 126 checks passed
@jkotas jkotas deleted the copilot/fix-115853 branch May 27, 2025 20:18
@github-actions github-actions bot locked and limited conversation to collaborators Jun 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CORCOMPILE_FIXUP_BLOB_KIND is redundant code
5 participants