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

Create armv4t_none_eabi.rs #99226

Closed
wants to merge 11 commits into from
Closed

Create armv4t_none_eabi.rs #99226

wants to merge 11 commits into from

Conversation

Lokathor
Copy link
Contributor

@Lokathor Lokathor commented Jul 14, 2022

This adds a target description for the armv4t-none-eabi target, intended as a new Tier 3 target.

This target is essentially intended to be nearly identical to thumbv4t-none-eabi, except that functions codegen as a32 code by default instead of using t32. Said another way: the thumb CPU feature is off by default for this target.

This is not intended to be related to the existing armv4t-unknown-linux-gnueabi target. I haven't spoken with them at all, but that's for a linux device, and this is a bare metal device.


armv4t-none-eabi target quiz

A tier 3 target must have a designated developer or developers (the "target maintainers") on record to be CCed when issues arise regarding the target.

That's me!

Targets must use naming consistent with any existing targets

We're using the existing name as recognized by LLVM and GCC

Tier 3 targets may have unusual requirements to build or use, but must not create legal issues or impose onerous legal terms for the Rust project or for Rust developers or users.

No legal issues here.

The target must not introduce license incompatibilities.

No license requirements here.

Anything added to the Rust repository must be under the standard Rust license (MIT OR Apache-2.0).

check

The target must not cause the Rust tools or libraries built for any other host (even when supporting cross-compilation to the target) to depend on any new dependency less permissive than the Rust licensing policy.

no new deps, we're just adding a rustc target description file for a target llvm already knows about.

Compiling, linking, and emitting functional binaries, libraries, or other code for the target (whether hosted on the target itself or cross-compiling from another target) must not depend on proprietary (non-FOSS) libraries.

bare-metal target, doesn't rely on any libs at all.

Tier 3 targets should attempt to implement as much of the standard libraries as possible and appropriate

core only here. You could build alloc too, but you'd have to bring your own global allocator.

The target must provide documentation for the Rust community explaining how to build for the target, using cross-compilation if possible.

LLVM knows how to do it, you just need the GNU Binutils linker because LLVM's linker doesn't work that far back. That's in the docs as part of this PR.

Tier 3 targets must not impose burden on the authors of pull requests, or other developers in the community, to maintain the target.

No burdens, LLVM already knows how to do this. Further, because this is a cpu-feature variant of an existing tier3 target the compiler-builtins crate has already been updated as necessary to fix any missing builtin function gaps.

Patches adding or updating tier 3 targets must not break any existing tier 2 or tier 1 target, and must not knowingly break another tier 3 target without approval of either the compiler team or the maintainers of the other tier 3 target.

check.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 14, 2022
@rust-highfive
Copy link
Collaborator

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 14, 2022
@Urgau
Copy link
Member

Urgau commented Jul 14, 2022

To speed up the review process of this PR, can you add to your initial message the answers to the Tier 3 Target Tier Policy. Example here #94872 (comment). As well as adding an entry in src/doc/rustc/src/platform-support, example here: https://github.com/rust-lang/rust/pull/94872/files#diff-d320f0b01f351d17f71e524691dca8850180c49d04913eef532d6cfc6469a09e

@Lokathor
Copy link
Contributor Author

@Urgau docs updated.

@rust-log-analyzer

This comment has been minimized.

@LunarLambda
Copy link

LunarLambda commented Jul 15, 2022

Does mixing targets work? I.e. building core with a32, but other code with t32?

What about crates that want to default to a32, but downstream is t32? Will the crates all get built as t32?

Would it be more reasonable to only have one target, and instead allow the instruction_set attribute to be used at the crate level?

Maybe I'm misunderstanding, but I believe the thumb* targets are mostly intended for chips that ONLY support thumb code (i.e. ARMv7-M things)? I'm not really sure why some targets are only arm* and some are only thumb* otherwise, even when some (most?) of them would support both anyway.

@Lokathor
Copy link
Contributor Author

Lokathor commented Jul 15, 2022

Excellent questions

Does mixing targets work? I.e. building core with a32, but other code with t32?

Yes, sorta.

The linker handles it just fine. At the moment the compiler doesn't do this at all. But if you manually tell the compiler to link in a t32 library to your primarily a32 program, then linker will put it together just fine.

Given that newer devices don't use interworking like this, I think that it's extremely unlikely that the compiler will learn how to casually mix code from two different targets any time soon.

What about crates that want to default to a32, but downstream is t32? Will the crates all get built as t32?

All crates are built according to the target you build them for, not their own wishes. If a crate is only appropriate on some targets (eg: it uses explicit sse2 intrinsics and so it can't run on ARM) then the crate needs to handle that itself. This has always been the case.

Would it be more reasonable to only have one target, and instead allow the instruction_set attribute to be used at the crate level?

While that's a neat idea, it wouldn't help in this case. The core crate needs to be built for one of a32 or t32 before any other crate gets built. And functions will generally not inline across cpu-feature bounds, so if core is t32 and you write an a32 function, (nearly) no core function/method will inline into your function. Rust relies on inlining more than most people even imagine, and it really sucks when even the index operator can generate actual function calls in optimized code.

(the exception to this is certain limited cases where rustc will inline before handing things to llvm, so a particular + call might be inlined for example, since rust handles integer + itself and doesn't actually call the add method.)

Separately, a person might not be interested in using thumb code at all. It's entirely possible to just use a32 code the entire program.

Maybe I'm misunderstanding, but I believe the thumb* targets are mostly intended for chips that ONLY support thumb code (i.e. ARMv7-M things)? I'm not really sure why some targets are only arm* and some are only thumb* otherwise, even when some (most?) of them would support both anyway.

Yes: Not only are there some CPUs that support arm/thumb interworking, but there's also CPUs that came out after that which only support thumb mode. (Technically it's an extension of thumb called "thumb2", but the point is that such CPUs don't support ARM mode at all.)

  • If your target CPU supports interworking, arm or thumb sets the default for a function, but also you can change it on a per-function basis if you need to (using the instruction_set attribute).
  • If your CPU does not support interworking, arm or thumb set the only mode your functions are allowed to use.

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 29, 2022
…me-pointer, r=davidtwco

Fix thumbv4t-none-eabi frame pointer setting

The `thumb_base` profile has changed since I last remember seeing it, and now it sets the frame pointer to "always keep", which is not desired for this target. Hooking a debugger to the running program is not really done, it's preferable to have the register available for actual program use, so the default "may omit" is now set.

I thought that the target was already using "may omit" when I checked on it last month, because I forgot that the target was previously based on `thumb_base` rather than `Default::default()`. I only noticed the issue just now when creating the `armv4t-none-eabi` target (rust-lang#99226), though this PR is not in any way conditional on that one.
@Lokathor
Copy link
Contributor Author

Lokathor commented Aug 5, 2022

@jackh726 ping

@jackh726
Copy link
Member

jackh726 commented Aug 7, 2022

@Lokathor can you squash the commits? There's a merge commit in there

@Lokathor
Copy link
Contributor Author

Lokathor commented Aug 7, 2022

i can but this'll take a while because i'll have to clone the repo

@Lokathor
Copy link
Contributor Author

Lokathor commented Aug 7, 2022

uh, apparently i cant? when i try to squash them together with the desktop program It's telling me

Unable to squash. Squashing replays all commits up to the last one required for the squash. A merge commit cannot exist among those commits.

I have no idea how git works though so maybe some other cli thing can still do it?

@jackh726
Copy link
Member

jackh726 commented Aug 7, 2022

hmm

@Lokathor
Copy link
Contributor Author

Lokathor commented Aug 7, 2022

I just made #100244 instead.

@Lokathor Lokathor closed this Aug 7, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 7, 2022
…2, r=jackh726

Add armv4t-none-eabi take2

This is the same as the previous PR (rust-lang#99226) but i just made a fresh branch without a merge commit in it.

---

### armv4t-none-eabi target quiz

> A tier 3 target must have a designated developer or developers (the "target maintainers") on record to be CCed when issues arise regarding the target.

That's me!

> Targets must use naming consistent with any existing targets

We're using the existing name as recognized by LLVM and GCC

> Tier 3 targets may have unusual requirements to build or use, but must not create legal issues or impose onerous legal terms for the Rust project or for Rust developers or users.

No legal issues here.

>> The target must not introduce license incompatibilities.

No license requirements here.

>> Anything added to the Rust repository must be under the standard Rust license (MIT OR Apache-2.0).

check

>> The target must not cause the Rust tools or libraries built for any other host (even when supporting cross-compilation to the target) to depend on any new dependency less permissive than the Rust licensing policy.

no new deps, we're just adding a rustc target description file for a target llvm already knows about.

>> Compiling, linking, and emitting functional binaries, libraries, or other code for the target (whether hosted on the target itself or cross-compiling from another target) must not depend on proprietary (non-FOSS) libraries.

bare-metal target, doesn't rely on any libs at all.

> Tier 3 targets should attempt to implement as much of the standard libraries as possible and appropriate

`core` only here. You could build `alloc` too, but you'd have to bring your own global allocator.

> The target must provide documentation for the Rust community explaining how to build for the target, using cross-compilation if possible.

LLVM knows how to do it, you just need the GNU Binutils linker because LLVM's linker doesn't work that far back. That's in the docs as part of this PR.

> Tier 3 targets must not impose burden on the authors of pull requests, or other developers in the community, to maintain the target.

No burdens, LLVM already knows how to do this. Further, because this is a cpu-feature variant of an existing tier3 target the `compiler-builtins` crate has already been updated as necessary to fix any missing builtin function gaps.

> Patches adding or updating tier 3 targets must not break any existing tier 2 or tier 1 target, and must not knowingly break another tier 3 target without approval of either the compiler team or the maintainers of the other tier 3 target.

check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants