Skip to content
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

Consistent naming for Arm "thumb" targets? #44722

Open
parched opened this issue Sep 20, 2017 · 10 comments
Open

Consistent naming for Arm "thumb" targets? #44722

parched opened this issue Sep 20, 2017 · 10 comments
Labels
C-bug Category: This is a bug. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-embedded Working group: Embedded systems

Comments

@parched
Copy link
Contributor

parched commented Sep 20, 2017

This commit changed the armv7-linux-androideabi target from generating Arm code to Thumb code. This may be deemed a valid change but I feel like it deserves at least a short discussion for a number of reasons.

  • This is a breaking change for anyone that was relying on that target generating Arm code. e.g., anyone using inline assembly with Arm state only instructions or someone bit fiddling code pointers and maybe some other ways. However, I think it's unlikely someone was doing this.
  • There is a convention for the rust target names that generate Thumb code to start with thumb instead of arm. This is the same convention clang uses for --target and LLVM.

Either way, the same change should probably be applied to all the armv7* targets for consistency.

@TimNN TimNN added C-bug Category: This is a bug. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 20, 2017
@parched
Copy link
Contributor Author

parched commented Sep 28, 2017

Anyone have any thoughts on this? @alexcrichton ? @japaric who added the other thumb targets maybe?

@alexcrichton
Copy link
Member

Is this a breaking change? It's specified as supporting thumb, so wasn't this just a bug fix?

@parched
Copy link
Contributor Author

parched commented Sep 29, 2017

@alexcrichton No not a bug fix, more of a change to something different. All the Arm targets support Thumb, that link is just saying if you're in Thumb state the Thumb2 extensions are available for that target. It's up to the developer to choose whether they want to generate Arm or Thumb code, e.g, with gcc you would use -marm or -mthumb. It's a trade off between a more complete instruction set and better code density. However Thumb state with Thumb2 extensions covers most of the Arm instruction set so generally you should pick that.

My personal opinion is we should add thumb variants of all the arm targets or at least the Thumb2 ones.

@alexcrichton
Copy link
Member

@parched I think you may be misunderstanding the change here? This was just a change for specifically the armv7 Android target (no other armv7 target). The actual definition for armv7 Android (not other armv7 targets, just Android) defines thumb/thumb2 available for use, so this was just a bugfix to actually use it.

@parched
Copy link
Contributor Author

parched commented Oct 2, 2017

@alexcrichton I think I see what you're saying, armv7-linux-androideabi was the only target that explicitly had +thumb2 and the intentional all along was for it to generate Thumb code. The +thumb2 is actually redundant as that is a default feature for all armv7* targets. My question is then, why this target should generate Thumb code but none of the other arm* targets should?

@alexcrichton
Copy link
Member

Indeed yeah with thumb/armv7. I don't know why other armv7 targets wouldn't generate thumb, I'm probably not the best person to ask for that (unsure who is)

@parched
Copy link
Contributor Author

parched commented Oct 6, 2017

@alexcrichton Would you be apposed to have both armv7-linux-androideabi and thumbv7-linux-androideabi as well as armv7-unknown-linux-gnueabihf and thumbv7-unknown-linux-gnueabihf targets? Ultimately I think we want to give the user the choice between Arm and Thumb for every target but adding all those extra targets would probably be too much of a burden until Cargo does on demand std compilation. Although, when that happens, using -C target-feature could be used to change everything.

My main concern is I think there should be a consistent default for all the Arm targets to whether they generate Arm or Thumb code. Or do we say it's unspecified and you have to always be explicit with -C target-feature if you care?

@alexcrichton
Copy link
Member

I'm probably the wrong person to be asking these questions, I just r+'d the PR which we defined to be correct, I don't know enough about ARM/thumb to make decisions about what targets we should be providing.

@parched
Copy link
Contributor Author

parched commented Nov 14, 2017

FWIW here's another place that relies on the name of the target to tell whether to generate Thumb code https://github.com/rust-lang-nursery/compiler-builtins/blob/bb2c81b700440f3b1d8a9544d3d33f5454a730f7/build.rs#L4279

@Enselic
Copy link
Member

Enselic commented Sep 14, 2023

Triage: As this issue is pretty old and presumably lots of changed, I would like to check. @parched Do you still miss any target in this list from Rust 1.72? If yes, which ones?

$ rustc +stable --print target-list | grep -e arm -e thumb
arm-linux-androideabi
arm-unknown-linux-gnueabi
arm-unknown-linux-gnueabihf
arm-unknown-linux-musleabi
arm-unknown-linux-musleabihf
arm64_32-apple-watchos
armeb-unknown-linux-gnueabi
armebv7r-none-eabi
armebv7r-none-eabihf
armv4t-none-eabi
armv4t-unknown-linux-gnueabi
armv5te-none-eabi
armv5te-unknown-linux-gnueabi
armv5te-unknown-linux-musleabi
armv5te-unknown-linux-uclibceabi
armv6-unknown-freebsd
armv6-unknown-netbsd-eabihf
armv6k-nintendo-3ds
armv7-apple-ios
armv7-linux-androideabi
armv7-sony-vita-newlibeabihf
armv7-unknown-freebsd
armv7-unknown-linux-gnueabi
armv7-unknown-linux-gnueabihf
armv7-unknown-linux-musleabi
armv7-unknown-linux-musleabihf
armv7-unknown-linux-ohos
armv7-unknown-linux-uclibceabi
armv7-unknown-linux-uclibceabihf
armv7-unknown-netbsd-eabihf
armv7-wrs-vxworks-eabihf
armv7a-kmc-solid_asp3-eabi
armv7a-kmc-solid_asp3-eabihf
armv7a-none-eabi
armv7a-none-eabihf
armv7k-apple-watchos
armv7r-none-eabi
armv7r-none-eabihf
armv7s-apple-ios
thumbv4t-none-eabi
thumbv5te-none-eabi
thumbv6m-none-eabi
thumbv7a-pc-windows-msvc
thumbv7a-uwp-windows-msvc
thumbv7em-none-eabi
thumbv7em-none-eabihf
thumbv7m-none-eabi
thumbv7neon-linux-androideabi
thumbv7neon-unknown-linux-gnueabihf
thumbv7neon-unknown-linux-musleabihf
thumbv8m.base-none-eabi
thumbv8m.main-none-eabi
thumbv8m.main-none-eabihf

@workingjubilee workingjubilee changed the title armv7-linux-androideabi target now generates Thumb code Maybe we should have a scheme for Arm "thumb" targets? Oct 26, 2024
@workingjubilee workingjubilee changed the title Maybe we should have a scheme for Arm "thumb" targets? Consistent naming for Arm "thumb" targets? Oct 26, 2024
@workingjubilee workingjubilee added the WG-embedded Working group: Embedded systems label Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-embedded Working group: Embedded systems
Projects
None yet
Development

No branches or pull requests

5 participants