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

Add initial support for m68k #88321

Merged
merged 12 commits into from
Sep 20, 2021
Merged

Add initial support for m68k #88321

merged 12 commits into from
Sep 20, 2021

Conversation

glaubitz
Copy link
Contributor

This patch series adds initial support for m68k making use of the new M68k
backend introduced with LLVM-13. Additional changes will be needed to be
able to actually use the backend for this target.

@rust-highfive
Copy link
Collaborator

r? @wesleywiser

(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 Aug 25, 2021
@glaubitz
Copy link
Contributor Author

CC @ricky26

@rust-log-analyzer

This comment has been minimized.

@glaubitz
Copy link
Contributor Author

Hmm, that LLVM build error on (x86_64-gnu-tools, 1, ubuntu-latest-xl) is odd.

@ricky26 Any idea?

@rust-log-analyzer

This comment has been minimized.

@glaubitz
Copy link
Contributor Author

Hmm, odd. My local builds work fine with gcc-7.5 on openSUSE 15.3. I'll try Debian Bullseye with gcc-10.

@ricky26
Copy link
Contributor

ricky26 commented Aug 25, 2021

It looks like it's due to Kind being both an enum class type & a field in that struct. Must be a compiler version thing though, since it does compile fine over here (and I'm pretty sure at least one of the LLVM bots builds the M68k target).

@rust-log-analyzer

This comment has been minimized.

@glaubitz
Copy link
Contributor Author

@ricky26 This looks eerily similar to this issue: intel/gazebo-sitl#46

And it might be this particular bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60994

I'll research whether the gcc in Ubuntu 18.04 LTS is still affected by the bug.

Target {
llvm_target: "m68k-unknown-linux-gnu".to_string(),
pointer_width: 32,
data_layout: "E-m:e-p:32:32-i8:8:8-i16:16:16-i32:32:32-n8:16:32-a:0:32-S16".to_string(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This data layout is wrong, the m68k ABI in use doesn't have 32-bit-aligned ints etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this should be it then?

glaubitz@node54:/data/home/glaubitz/llvm-project/build> clang -S -x c /dev/null -o - -emit-llvm --target=m68k-linux-gnu | grep datalayout
target datalayout = "E-m:e-p:32:32-i8:8:8-i16:16:16-i32:16:32-n8:16:32-a:0:16-S16"
glaubitz@node54:/data/home/glaubitz/llvm-project/build>

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p:32:32 still looks wrong, _Alignof(void *) is 2 not 4 so it should be p:32:16:32 just like i32, so that needs fixing in LLVM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, we'll have to report that to the LLVM bug tracker then.

Any idea regarding the build failure?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repeating what I said on IRC here so others can see:
[20:06:26] -- The C compiler identification is GNU 5.4.0
[20:06:31] ditto CXX
[20:08:14] the job is using a 16.04 environment
[20:09:23] the host is 18.04 but the builds are done in docker containers
[20:12:31] (by running docker build on https://github.com/rust-lang/rust/blob/master/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, yes, this looks like that GCC bug, and I suggest renaming the enum type itself to KindTy in the M68k AsmParser like is used in the RISCV one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repeating what I said on IRC here so others can see:
[20:06:26] -- The C compiler identification is GNU 5.4.0
[20:06:31] ditto CXX
[20:08:14] the job is using a 16.04 environment
[20:09:23] the host is 18.04 but the builds are done in docker containers
[20:12:31] (by running docker build on https://github.com/rust-lang/rust/blob/master/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile)

Ah, I was looking everywhere in the build log but I couldn't find the reference. But yeah, the Dockerfile wholly explains why we're on such an old compiler version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, yes, this looks like that GCC bug, and I suggest renaming the enum type itself to KindTy in the M68k AsmParser like is used in the RISCV one.

Would that work-around the issue? I assume yes when looking at what @ricky26 said.

@ricky26
Copy link
Contributor

ricky26 commented Aug 25, 2021

I've put a prospective patch in to LLVM: https://reviews.llvm.org/D108723.

Even if that lands in main soon, I don't know what the timeline would be for it getting into a release. (I'd be surprised if it qualified to go into the release branch.)

@jrtc27
Copy link

jrtc27 commented Aug 25, 2021

I've put a prospective patch in to LLVM: https://reviews.llvm.org/D108723.

Even if that lands in main soon, I don't know what the timeline would be for it getting into a release. (I'd be surprised if it qualified to go into the release branch.)

Rust uses its own fork of LLVM, https://github.com/rust-lang/llvm-project, precisely so it can cherry-pick upstream fixes. Distributions like Debian make Rust link against the system LLVM but will ensure required bug fixes are present in its system LLVM. What the upstream release branches have isn't generally relevant here for things like this.

@joshtriplett
Copy link
Member

joshtriplett commented Aug 25, 2021

This will need compiler-team approval for adding m68k-unknown-linux-gnu as a tier 3 target. (Are you volunteering to be the target maintainer of record?) The standard way of doing that is to copy the Tier 3 policy and respond to each bullet point.

This also needs to patch platform-support.md to list the new target.

@glaubitz
Copy link
Contributor Author

This will need compiler-team approval for adding m68k-unknown-linux-gnu as a tier 3 target. (Are you volunteering to be the target maintainer of record?) The standard way of doing that is to copy the Tier 3 policy and respond to each bullet point.

Yes, I'm willing to be the maintainer.

This also needs to patch platform-support.md to list the new target.

OK, I will add that.

@rust-log-analyzer

This comment has been minimized.

Target {
llvm_target: "m68k-unknown-linux-gnu".to_string(),
pointer_width: 32,
data_layout: "E-m:e-p:32:32-i8:8:8-i16:16:16-i32:16:32-n8:16:32-a:0:16-S16".to_string(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
data_layout: "E-m:e-p:32:32-i8:8:8-i16:16:16-i32:16:32-n8:16:32-a:0:16-S16".to_string(),
data_layout: "E-m:e-p:32:16:32-i8:8:8-i16:16:16-i32:16:32-n8:16:32-a:0:16-S16".to_string(),

Copy link

@jrtc27 jrtc27 Aug 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have guessed that LLVM would shout at you if it doesn't match what the backend thinks, but perhaps not, maybe that's specific to when Clang and LLVM disagree, given that some of you have clearly already built m68k rust binaries with the older version of this PR that had i32 also incorrectly-aligned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we fixed the data_layout on the LLVM side yet?

@@ -165,7 +165,7 @@ impl Step for Llvm {

let llvm_exp_targets = match builder.config.llvm_experimental_targets {
Some(ref s) => s,
None => "AVR",
None => { "AVR;M68k" }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why the { } are needed, it's still a single string literal?

@glaubitz
Copy link
Contributor Author

Hmm, odd that it still fails. I thought @ricky26 's patch got backported into the rustc LLVM fork already.

@rust-log-analyzer

This comment has been minimized.

@jrtc27
Copy link

jrtc27 commented Aug 26, 2021

You'll need to bump the submodule

Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you!

@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 17, 2021

📌 Commit 5d22b1a has been approved by wesleywiser

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 17, 2021
@Mark-Simulacrum
Copy link
Member

Just wanted to drop a thanks for the detailed docs in src/doc/rustc/src/platform-support/m68k-unknown-linux-gnu.md, those look great. ❤️

@glaubitz
Copy link
Contributor Author

Just wanted to drop a thanks for the detailed docs in src/doc/rustc/src/platform-support/m68k-unknown-linux-gnu.md, those look great. heart

Glad they're useful. Appreciate the positive feedback!

@bors
Copy link
Contributor

bors commented Sep 20, 2021

⌛ Testing commit 5d22b1a with merge db1fb85...

@bors
Copy link
Contributor

bors commented Sep 20, 2021

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing db1fb85 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 20, 2021
@bors bors merged commit db1fb85 into rust-lang:master Sep 20, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 20, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (db1fb85): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@reinvantveer
Copy link

Congrats and thanks @glaubitz @ricky26 for making this happen. Retro-enthusiasts around the world rejoice!

file \
curl \
ca-certificates \
python2.7 \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is python2.7 really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not. I wrote this Docker file over a year ago when the CI was still using an older version of Ubuntu.

I will verify that the issue and send in a follow-up PR to drop python2.7 if necessary.

Thanks for spotting this.

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Dec 6, 2021
Pkgsrc changes:
 * Adapt a couple of patches

Upstream changes:

Version 1.57.0 (2021-12-02)
==========================

Language
--------

- [Macro attributes may follow `#[derive]` and will see the original
  (pre-`cfg`) input.][87220]
- [Accept curly-brace macros in expressions, like `m!{ .. }.method()`
  and `m!{ .. }?`.][88690]
- [Allow panicking in constant evaluation.][89508]

Compiler
--------

- [Create more accurate debuginfo for vtables.][89597]
- [Add `armv6k-nintendo-3ds` at Tier 3\*.][88529]
- [Add `armv7-unknown-linux-uclibceabihf` at Tier 3\*.][88952]
- [Add `m68k-unknown-linux-gnu` at Tier 3\*.][88321]
- [Add SOLID targets at Tier 3\*:][86191] `aarch64-kmc-solid_asp3`,
  `armv7a-kmc-solid_asp3-eabi`, `armv7a-kmc-solid_asp3-eabihf`

\* Refer to Rust's [platform support page][platform-support-doc] for more
   information on Rust's tiered platform support.

Libraries
---------

- [Avoid allocations and copying in `Vec::leak`][89337]
- [Add `#[repr(i8)]` to `Ordering`][89507]
- [Optimize `File::read_to_end` and `read_to_string`][89582]
- [Update to Unicode 14.0][89614]
- [Many more functions are marked `#[must_use]`][89692], producing a warning
  when ignoring their return value. This helps catch mistakes such as expecting
  a function to mutate a value in place rather than return a new value.

Stabilised APIs
---------------

- [`[T; N]::as_mut_slice`][`array::as_mut_slice`]
- [`[T; N]::as_slice`][`array::as_slice`]
- [`collections::TryReserveError`]
- [`HashMap::try_reserve`]
- [`HashSet::try_reserve`]
- [`String::try_reserve`]
- [`String::try_reserve_exact`]
- [`Vec::try_reserve`]
- [`Vec::try_reserve_exact`]
- [`VecDeque::try_reserve`]
- [`VecDeque::try_reserve_exact`]
- [`Iterator::map_while`]
- [`iter::MapWhile`]
- [`proc_macro::is_available`]
- [`Command::get_program`]
- [`Command::get_args`]
- [`Command::get_envs`]
- [`Command::get_current_dir`]
- [`CommandArgs`]
- [`CommandEnvs`]

These APIs are now usable in const contexts:

- [`hint::unreachable_unchecked`]

Cargo
-----

- [Stabilize custom profiles][cargo/9943]

Compatibility notes
-------------------

Internal changes
----------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of rustc
and related tools.

- [Added an experimental backend for codegen with `libgccjit`.][87260]

[86191]: rust-lang/rust#86191
[87220]: rust-lang/rust#87220
[87260]: rust-lang/rust#87260
[88243]: rust-lang/rust#88243
[88321]: rust-lang/rust#88321
[88529]: rust-lang/rust#88529
[88690]: rust-lang/rust#88690
[88952]: rust-lang/rust#88952
[89337]: rust-lang/rust#89337
[89507]: rust-lang/rust#89507
[89508]: rust-lang/rust#89508
[89582]: rust-lang/rust#89582
[89597]: rust-lang/rust#89597
[89614]: rust-lang/rust#89614
[89692]: rust-lang/rust#89692
[cargo/9943]: rust-lang/cargo#9943
[`array::as_mut_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_mut_slice
[`array::as_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_slice
[`collections::TryReserveError`]: https://doc.rust-lang.org/std/collections/struct.TryReserveError.html
[`HashMap::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_map/struct.HashMap.html#method.try_reserve
[`HashSet::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_set/struct.HashSet.html#method.try_reserve
[`String::try_reserve`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve
[`String::try_reserve_exact`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve_exact
[`Vec::try_reserve`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve
[`Vec::try_reserve_exact`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve_exact
[`VecDeque::try_reserve`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve
[`VecDeque::try_reserve_exact`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve_exact
[`Iterator::map_while`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.map_while
[`iter::MapWhile`]: https://doc.rust-lang.org/std/iter/struct.MapWhile.html
[`proc_macro::is_available`]: https://doc.rust-lang.org/proc_macro/fn.is_available.html
[`Command::get_program`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_program
[`Command::get_args`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_args
[`Command::get_envs`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_envs
[`Command::get_current_dir`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_current_dir
[`CommandArgs`]: https://doc.rust-lang.org/std/process/struct.CommandArgs.html
[`CommandEnvs`]: https://doc.rust-lang.org/std/process/struct.CommandEnvs.html
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 22, 2022
Pkgsrc changes:
 * Adjust line numbers in a number of patches
 * remove the --disable-dist-src option, so that we produce
   the rust-src rust component, which we upload to LOCALSRC
   to allow the rust-src package to build, which is needed
   for rust-analyzer.
 * Cargo checksum for vendor/cc no longer needs patching;
   checksum for vendor/libc updated

Upstream changes:

Version 1.57.0 (2021-12-02)
==========================

Language
--------

- [Macro attributes may follow `#[derive]` and will see the original
  (pre-`cfg`) input.][87220]
- [Accept curly-brace macros in expressions, like `m!{ .. }.method()`
  and `m!{ .. }?`.][88690]
- [Allow panicking in constant evaluation.][89508]

Compiler
--------

- [Create more accurate debuginfo for vtables.][89597]
- [Add `armv6k-nintendo-3ds` at Tier 3\*.][88529]
- [Add `armv7-unknown-linux-uclibceabihf` at Tier 3\*.][88952]
- [Add `m68k-unknown-linux-gnu` at Tier 3\*.][88321]
- [Add SOLID targets at Tier 3\*:][86191] `aarch64-kmc-solid_asp3`,
  `armv7a-kmc-solid_asp3-eabi`, `armv7a-kmc-solid_asp3-eabihf`

\* Refer to Rust's [platform support page][platform-support-doc] for more
   information on Rust's tiered platform support.

Libraries
---------

- [Avoid allocations and copying in `Vec::leak`][89337]
- [Add `#[repr(i8)]` to `Ordering`][89507]
- [Optimize `File::read_to_end` and `read_to_string`][89582]
- [Update to Unicode 14.0][89614]
- [Many more functions are marked `#[must_use]`][89692], producing a warning
  when ignoring their return value. This helps catch mistakes such as expecting
  a function to mutate a value in place rather than return a new value.

Stabilised APIs
---------------

- [`[T; N]::as_mut_slice`][`array::as_mut_slice`]
- [`[T; N]::as_slice`][`array::as_slice`]
- [`collections::TryReserveError`]
- [`HashMap::try_reserve`]
- [`HashSet::try_reserve`]
- [`String::try_reserve`]
- [`String::try_reserve_exact`]
- [`Vec::try_reserve`]
- [`Vec::try_reserve_exact`]
- [`VecDeque::try_reserve`]
- [`VecDeque::try_reserve_exact`]
- [`Iterator::map_while`]
- [`iter::MapWhile`]
- [`proc_macro::is_available`]
- [`Command::get_program`]
- [`Command::get_args`]
- [`Command::get_envs`]
- [`Command::get_current_dir`]
- [`CommandArgs`]
- [`CommandEnvs`]

These APIs are now usable in const contexts:

- [`hint::unreachable_unchecked`]

Cargo
-----

- [Stabilize custom profiles][cargo/9943]

Compatibility notes
-------------------

Internal changes
----------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of rustc
and related tools.

- [Added an experimental backend for codegen with `libgccjit`.][87260]

[86191]: rust-lang/rust#86191
[87220]: rust-lang/rust#87220
[87260]: rust-lang/rust#87260
[88243]: rust-lang/rust#88243
[88321]: rust-lang/rust#88321
[88529]: rust-lang/rust#88529
[88690]: rust-lang/rust#88690
[88952]: rust-lang/rust#88952
[89337]: rust-lang/rust#89337
[89507]: rust-lang/rust#89507
[89508]: rust-lang/rust#89508
[89582]: rust-lang/rust#89582
[89597]: rust-lang/rust#89597
[89614]: rust-lang/rust#89614
[89692]: rust-lang/rust#89692
[cargo/9943]: rust-lang/cargo#9943
[`array::as_mut_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_mut_slice
[`array::as_slice`]: https://doc.rust-lang.org/std/primitive.array.html#method.as_slice
[`collections::TryReserveError`]: https://doc.rust-lang.org/std/collections/struct.TryReserveError.html
[`HashMap::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_map/struct.HashMap.html#method.try_reserve
[`HashSet::try_reserve`]: https://doc.rust-lang.org/std/collections/hash_set/struct.HashSet.html#method.try_reserve
[`String::try_reserve`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve
[`String::try_reserve_exact`]: https://doc.rust-lang.org/alloc/string/struct.String.html#method.try_reserve_exact
[`Vec::try_reserve`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve
[`Vec::try_reserve_exact`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve_exact
[`VecDeque::try_reserve`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve
[`VecDeque::try_reserve_exact`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.try_reserve_exact
[`Iterator::map_while`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.map_while
[`iter::MapWhile`]: https://doc.rust-lang.org/std/iter/struct.MapWhile.html
[`proc_macro::is_available`]: https://doc.rust-lang.org/proc_macro/fn.is_available.html
[`Command::get_program`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_program
[`Command::get_args`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_args
[`Command::get_envs`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_envs
[`Command::get_current_dir`]: https://doc.rust-lang.org/std/process/struct.Command.html#method.get_current_dir
[`CommandArgs`]: https://doc.rust-lang.org/std/process/struct.CommandArgs.html
[`CommandEnvs`]: https://doc.rust-lang.org/std/process/struct.CommandEnvs.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.