Only root supported hwintrinsic methods for interpreter-based platforms #123043
+1,139
−84
Conversation
This file contains hidden or 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
We don't need to root the IsSupported getters as the interpreter already handles those as `return false;`. Add a new helper in InstructionSetParser to do the "instruction set -> types that provide hardware intrinsic methods for said instruction set" mapping so we don't need to search every type in the system module now that we only need to go through the explicitly supported instruction sets.
Document definearch command in instructionsetdesc.txt Drive the "intrinsic namespace" logic for `LookupPlatformIntrinsicInstructionSet` from the newly added field in definearch. Fix the ARM64 intrinsics namespace.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the hardware intrinsic rooting logic for interpreter-based platforms by only rooting methods from supported instruction sets rather than all hardware intrinsic types in the system module.
Key changes:
- Added managed namespace parameter to architecture definitions in InstructionSetDesc.txt
- Created new
LookupPlatformIntrinsicTypeshelper method to map instruction sets to their managed types - Updated ReadyToRunHardwareIntrinsicRootProvider to iterate only supported instruction sets
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/tools/Common/JitInterface/ThunkGenerator/InstructionSetDesc.txt | Added 6th parameter for managed namespace to definearch commands; fixed alignment and removed managed names for internal-only instruction sets like VectorT128 and RiscV64 ISAs |
| src/coreclr/tools/Common/JitInterface/ThunkGenerator/InstructionSetGenerator.cs | Added _architectureManagedNamespace dictionary; updated definearch parsing to expect 6 parameters; generated new LookupPlatformIntrinsicTypes method with proper handling of nested types and 64-bit variants |
| src/coreclr/tools/Common/JitInterface/CorInfoInstructionSet.cs | Generated code: added using System; updated ArchitectureToValidInstructionSets to remove incorrect R2R names; added X64 case to LookupPlatformIntrinsicInstructionSet; implemented new LookupPlatformIntrinsicTypes method |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunHardwareIntrinsicRootProvider.cs | Replaced iteration over all system module types with efficient iteration over supported instruction sets using the new LookupPlatformIntrinsicTypes helper |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
We don't need to root the
IsSupportedgetters for unsupported intrinsic types as the interpreter already handles those asreturn false;.Add a new helper in
InstructionSetParserto do the "instruction set -> types that provide hardware intrinsic methods for said instruction set" mapping so we don't need to search every type in the system module now that we only need to go through the explicitly supported instruction sets.Adjust InstructionSetDesc.txt to support specifying the managed namespace for intrinsic instruction sets so we can better filter ISAs with no managed types (ie RiscV) and avoid hard-coding namespaces in
LookupPlatformIntrinsicInstructionSet.