Use the cpu given as --target/--host for rust

RESOLVED FIXED in Firefox 58

Status

defect
RESOLVED FIXED
3 years ago
5 months ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla58
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

3 years ago
Currently, we have a hardcoded list of rust targets, set depending on cpu family and OS.

The immediate problem is that since bug 1299864, Mozilla builds use a non-default rust target (i586-unknown-linux-gnu) to avoid SSE2 codegen from the normal i686 rust target.

But while doing that, it also enforces anyone building a x86 linux Firefox with rust enabled to have that target available, while a) it's likely unavailable b) it's likely not what the person building wants.

The least worse thing we can do IMHO is to derive the rust target from the C++ target, and make them both use the same "raw" CPU.

Which means to build with the i586-unknown-linux-gnu target on our (Mozilla) builds, we'd switch to --target=i586-linux-gnu instead of --target=i686-linux-gnu. People building with a --target=i686-linux-gnu would then pick the i686-unknown-linux-gnu target for rust.

I tested that on try, comparing the output with the current build, and there is no difference, except for the package file names, which can be worked around (and MOZ_BUILD_DATE).
Assignee

Updated

3 years ago
Depends on: 1319333
Isn't the i585-linux support requirement going away in firefox 53, with bug 1308167? Or is this about maintaining tier-3 build support for that target?

Note that you'll still need a cpu mapping table, unless you fix also our for C++/android arm vs rust armv7.
Assignee

Updated

3 years ago
Duplicate of this bug: 1320022
(In reply to Ralph Giles (:rillian) needinfo me from comment #1)
> Note that you'll still need a cpu mapping table, unless you fix also our for
> C++/android arm vs rust armv7.

And even GNU/Linux on ARMv7 using Mozilla-shipped Rust. E.g. Debian says armv7l-unknown-linux-gnueabihf bit Mozilla-shipped (rustup.rs) Rust wants armv7-unknown-linux-gnueabihf. (Notice no 'l' after '7'.)

It could be argued that this is a Rust bug, but anyway.
Assignee

Comment 4

3 years ago
We "just" need to correlate with the output from rustc --print target-list.

Comment 5

2 years ago
Hi, has been there any progress? We are running a CI system for tier-3 arches (ppc64/ppc64le, s390x) and it requires patching the sources before we can even attempt a build there.
Nobody has done any work here that I'm aware of. Per comment 4, the best way to fix this is probably:
1) add a function in rust.configure that runs `rustc --print target-list` and calls `split_triplet` on each line of output to canonicalize it. This will probably need some fixup because `split_triplet` currently errors on unknown OS/CPU values. Perhaps we could add an additional argument like `allow_unknown=True` that would simply set `canonical_os = os` and `canonical_cpu = cpu` for unknown values:
https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/build/moz.configure/init.configure#500

2) In `rust_target`, instead of the hardcoded lookup table, make a lookup table from the list of values returned from the previous step, probably still indexed by (cpu, os), like `targets = {(t.cpu, t.os): t.alias for t in rustc_targets}`:
https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/build/moz.configure/rust.configure#133
Comment hidden (mozreview-request)
Assignee

Comment 8

2 years ago
Comment on attachment 8915486 [details]
Bug 1319332 - Derive the rust targets from the build host/target.

Always fun when something works locally but not on try.
Attachment #8915486 - Flags: review?(nfroyd)
Comment hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8915486 [details]
Bug 1319332 - Derive the rust targets from the build host/target.

https://reviewboard.mozilla.org/r/186700/#review191850

::: build/moz.configure/rust.configure:159
(Diff revision 2)
>          # compiled for the appropriate ABI.  We need to perform appropriate
>          # munging to get the correct option to rustc.
>          #
>          # The canonical list of targets supported can be derived from:
>          #
>          # https://github.com/rust-lang/rust/blob/master/src/librustc_back/target/mod.rs

You can probably drop this reference now that you've added `rust_supported_targets` and just say that we get the list of supported targets from rustc and try to find the closest match to the autoconf-style target triple we're using.
Comment hidden (mozreview-request)

Comment 12

2 years ago
The proposed patch allows to detect Rust on Fedora 27 ppc64le and Fedora 26 s390x systems.

Comment 13

2 years ago
mozreview-review
Comment on attachment 8915486 [details]
Bug 1319332 - Derive the rust targets from the build host/target.

https://reviewboard.mozilla.org/r/186700/#review192880

This is nice.

::: build/moz.configure/rust.configure:162
(Diff revision 2)
> -        # they don't differ.
> -        os_or_kernel = host_or_target.kernel if host_or_target.kernel == 'Linux' and host_or_target.os != 'Android' else host_or_target.os
> -        # If we are targetting Windows but the compiler is gcc, we need to
> -        # use a different rustc target
> +            if building_with_gcc:
> +                host_or_target_raw_os = host_or_target_os = 'windows-gnu'
> +            else:
> +                host_or_target_raw_os = host_or_target_os = 'windows-msvc'

The double assignment here is a little subtle--I was almost going to comment on the absence of assignment to `host_or_target_os`.  Maybe:

```
if building_with_gcc:
  host_or_target_raw_os =
else:
  host_or_target_raw_os =
host_or_target_os = host_or_target_raw_os
```
?

::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:1421
(Diff revision 2)
> +            # Raw list returned by rustc version 1.19, + ios, which somehow
> +            # don't appear https://github.com/rust-lang/rust/issues/36156

Nit: "...which somehow doesn't appear in the default list"?

::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:1537
(Diff revision 2)
> +            'x86_64-unknown-openbsd',
> +            'aarch64-unknown-linux-gnu',
> +            'armv7-unknown-linux-gnueabihf',
> +            'sparc64-unknown-linux-gnu',
> +            'i686-unknown-linux-gnu',
> +            'x86_64-unknown-linux-gnu',

I think Fedora (maybe others?) use `x86_64-pc-linux-gnu`?  Does that get canonicalized by config.sub?
Attachment #8915486 - Flags: review?(nfroyd) → review+
Assignee

Comment 14

2 years ago
> I think Fedora (maybe others?) use `x86_64-pc-linux-gnu`?  Does that get canonicalized by config.sub?

No it's not, and indeed, while config.guess used to give x86_64-unknown-linux-gnu, it now returns x86_64-pc-linux-gnu. That said, the unknown/pc part is ignored entirely. I'll replace anyways.
Comment hidden (mozreview-request)

Comment 16

2 years ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/035d3125ca2f
Derive the rust targets from the build host/target. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/035d3125ca2f
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

Updated

a year ago
Product: Core → Firefox Build System
Duplicate of this bug: 1273456
You need to log in before you can comment on or make changes to this bug.