-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
[LLD][COFF] Add support for CHPE redirection metadata. #105739
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
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.
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.
Is this documented somewhere as being named with
CHPE
, as opposed to ARM64EC in some form?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'm not aware of this being explicitly documented. It's used in CHPE metadata, which is a somewhat public name; that's how it's referred to in the IMAGE_LOAD_CONFIG_DIRECTORY64 structure. The __chpe_metadata is somewhat public too - it’s explicitly referenced by the linker in the case of hybrid ARM64X images. These images have two separate load configurations but a single CHPE metadata, so the native load config can't directly reference the ARM64EC __chpe_metadata symbol, and the linker needs to look it up and use it to modify the load config instead.
Another place where this is named is in the __arm64x_redirection_metadata symbol from this PR. Note that it uses "arm64x," not "arm64ec." I think there is some ambiguity about what "arm64x" means. I usually use it to refer to hybrid files (as implied by -machine:arm64x) as ARM64X, while PE files containing only ARM64EC code (as implied by -machine:arm64ec) are referred to as ARM64EC. However, I'm no longer sure if this is the intended nomenclature. I’ve seen the ARM64X name used for both image types, so I guess ARM64X would refer to PE files containing ARM64EC code, while ARM64EC is the name of the ABI. As such, using ARM64X here would also be correct.
That said, I can see arguments for using any of the CHPE, ARM64EC, or ARM64X variants. I chose CHPE to emphasize that it's a part of CHPE metadata, but I wouldn't mind renaming it if you feel differently.
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.
Thanks, if this is referred to in the load configs with a CHPE prefix, using these names is totally fine with me.
(I guess historically, the naming may stem from which parts were used in CHPEv1 and which are new for ARM64EC etc.)