-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Delete FEATURE_MULTIREG_RETURN #116122
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
Delete FEATURE_MULTIREG_RETURN #116122
Conversation
It was made obsolete by earlier thread suspension changes
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.
Pull Request Overview
This PR removes the obsolete FEATURE_MULTIREG_RETURN functionality that is no longer needed due to earlier thread suspension changes.
- Removed FEATURE_MULTIREG_RETURN related comments and code in src/coreclr/vm/threads.h
- Updated union definitions in src/coreclr/vm/amd64/cgencpu.h to rely on UNIX_AMD64_ABI where applicable
- Removed the IsValidReturnRegister helper and FEATURE_MULTIREG_RETURN logic in src/coreclr/inc/gcinfotypes.h and clrdefinitions.cmake
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/coreclr/vm/threads.h | Removed obsolete comments regarding FEATURE_MULTIREG_RETURN |
src/coreclr/vm/amd64/cgencpu.h | Adjusted union configuration based on UNIX_AMD64_ABI |
src/coreclr/inc/gcinfotypes.h | Removed the IsValidReturnRegister function while retaining its invocation |
src/coreclr/clrdefinitions.cmake | Removed compile definitions for FEATURE_MULTIREG_RETURN |
Comments suppressed due to low confidence (1)
src/coreclr/inc/gcinfotypes.h:253
- The function IsValidReturnRegister has been removed, but its usage remains in the assertion. Please update or remove this assert to match the new logic.
_ASSERTE(IsValidReturnRegister(returnRegOrdinal));
Tagging subscribers to this area: @mangod9 |
endif() | ||
add_compile_definitions($<$<NOT:$<BOOL:$<TARGET_PROPERTY:IGNORE_DEFAULT_TARGET_ARCH>>>:FEATURE_MULTIREG_RETURN>) | ||
elseif (CLR_CMAKE_TARGET_ARCH_ARM) | ||
if (CLR_CMAKE_HOST_WIN32 AND NOT DEFINED CLR_CROSS_COMPONENTS_BUILD) |
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.
This was left-over from Win Arm32 that I have deleted as well.
At some point I thought that even |
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.
LGTM!
Co-authored-by: Theodore Tsirpanis <[email protected]>
/ba-g unrelated test failure without a log |
It was made obsolete by earlier thread suspension changes