Add support for legacy Armv4 and Armv4T architectures #61
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
- Added Armv4 and Armv4T variants to Arch enum with proper doc comments - Updated Arch::get, Arch::values, and Display to include new variants - Updated Arch::profile mapping to include Armv4/Armv4T - Updated Isa::get to assign correct instruction set to legacy architectures - Ensured TargetInfo populates arch, isa, and profile for all supported targets
|
This looks OK, but which ARMv4 processor were you thinking about here? Rust does not currently support ARMv4 (ARM8, StrongARM), only ARMv4T (ARM7TDMI). ARMv5TE might be more useful. |
- Removed Arch::Armv4 as Rust does not support it - Added Arch::Armv5TE for relevant target triples - Updated Isa::get() to map Armv4T and Armv5TE to A32
|
Hey @jonathanpallant thanks for putting time to my PR, and thanks for the feedback, I have added ARMv5TE and removed ARMv4 because as you said it's not supported by RUST :) If there are more improvements for the code, I'll definitely be happy to make them. |
- Introduced Profile::Legacy for ARMv4T and ARMv5TE as they predate A/R/M profiles - Updated TargetInfo and Arch::profile() mapping accordingly - Removed any references to unsupported ARMv4
|
Hey @jonathanpallant, thanks again for the feedback! I’ve added a Legacy profile for ARMv4T and ARMv5TE since they predate the A/R/M profiles, and removed ARMv4 references. Everything else remains the same. Happy to make more improvements if needed! |
korken89
previously requested changes
Sep 22, 2025
jonathanpallant
approved these changes
Sep 22, 2025
|
Thank you for these changes. |
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description:
This PR extends the Arm target helpers to include legacy architectures Armv4 and Armv4T.
Changes made:
Added Armv4 and Armv4T to the Arch enum along with documentation comments.
Updated Arch::get() to recognize armv4-* and armv4t-* target triples.
Updated Arch::values() and Display to include the new variants.
Updated Arch::profile() to map Armv4 and Armv4T to the Profile::R.
Updated Isa::get() to correctly assign A32 for the new legacy architectures.
Verified that TargetInfo now properly populates all fields (arch, isa, profile) for legacy and modern Arm targets.
TL;DR:
Previously, Armv4 and Armv4T targets were ignored, resulting in None values for arch and isa. This update ensures full support for legacy ARM targets, improving build script compatibility.
This PR addresses issue #10