Implement flexible target specification #16156

emberian · 2014-08-01T02:58:35Z

See commit message.

emberian · 2014-08-01T02:59:11Z

Still trying to figure out a bug where executables aren't getting the executable bit set...

emberian · 2014-08-01T07:25:43Z

Seems to have resolved itself with a rebase ¯_(ツ)_/¯

huonw · 2014-08-01T07:57:35Z

src/librustc_back/target/mod.rs

+        let mut paths = os::split_paths(target_path.as_slice());
+        // FIXME: should be relative to the prefix rustc is installed in, and do something
+        // different for Windows.
+        paths.push(Path::new("/etc/rustc"));


Who installs to /etc/rustc?

This feature may want to remain unimplemented for now due to some open questions here, but it probably doesn't matter too much.

huonw · 2014-08-01T07:59:34Z

Can you write a run-make test that checks the compiler is correctly searching for, parsing and using external specifications? (E.g. it could run rustc --target=foo-bar-baz --emit=asm ... with a custom target that disables stack checks, and then grep for stack checks.)

emberian · 2014-08-01T08:34:07Z

Another issue: probably needs to allow specifying target_os instead of taking it from the target triple. I'm not actually sure this is a good idea, though, and it seems MUCH more clear that the target_os comes from the triple name itself. But we'd need to rename "osx" to "darwin", which I feel is almost a lie? Not really sure. Need feedback.

emberian · 2014-08-01T08:34:21Z

(The RFC specifies taking it from the target)

emberian · 2014-08-01T09:05:17Z

@huonw how do those docs look?

emberian · 2014-08-01T09:11:38Z

Hm, glaringly obvious problem: linker should probably include a target triple prefix, for cross compiling. ie, x86_64-unknown-linux-gnu-cc.

errordeveloper · 2014-08-01T09:35:58Z

@farcaller we should try this with arm-none-eabi

emberian · 2014-08-01T09:44:23Z

Be warned that this isn't quite working yet. My current status is:

LLVM ERROR: not a number, or does not fit in an unsigned int

emberian · 2014-08-01T09:47:03Z

(Already fixed I think, still building..)

huonw · 2014-08-01T09:54:54Z

src/librustc_back/target/mod.rs

+//! will be given.  `RUST_TARGET_PATH` includes `/etc/rustc` as its last entry, to be searched by
+//! default.
+//!
+//! Projects defining their own targets should use `--target=path/to/my-awesome-platform.json`


Do we have a story for interacting with cargo?

Not yet, no.

huonw · 2014-08-01T09:58:22Z

src/librustc_back/target/mod.rs

+    pub code_model: String,
+    /// Do not emit code that uses the "red zone", if the ABI has one. Defaults to true.
+    pub disable_redzone: bool,
+    /// String to use as the `target_endian` `cfg` variable. Must be specified.


Is it crazy to reorder these so that the required fields are together at the start of the definition?

huonw · 2014-08-01T09:58:55Z

src/librustc_back/target/mod.rs

+//! Targets are defined using [JSON](http://json.org/). The `Target` struct in this module defines
+//! the format the JSON file should take, though each underscore in the field names should be
+//! replaced with a hyphen (`-`) in the JSON file. Some fields are required in every target
+//! specification, such as `data-layout`, `llvm-target`, `target-endian`, `target-word-size`, and


Could you explicitly state that most fields are optional?

alexcrichton · 2014-08-01T14:31:35Z

src/librustc/back/link.rs

-                    sess.abort_if_errors();
-                }
+    if sess.target.target.is_like_osx && sess.opts.debuginfo != NoDebugInfo {
+        match Command::new("dsymutil").arg(out_filename).status() {


status => output

alexcrichton · 2014-08-01T14:32:27Z

src/librustc/back/link.rs

-        }
+        let mut v = b"-Wl,-force_load,".to_vec();
+        v.push_all(morestack.as_vec());
+        cmd.arg(v.as_slice());
    }


This seems to have lost the fall-through case?

alexcrichton · 2014-08-01T14:34:44Z

src/librustc/driver/config.rs

+                Some(..) => false,
+                None => { *slot = Some(true); true }
+            }
+        }


This seems backwards?

Is parse_bool also backwards, then?

Ah nevermind, I remember what this is doing now. No worries!

alexcrichton · 2014-08-01T14:35:14Z

src/librustc/driver/config.rs

-        abi::Mips =>   ("big",    "mips",   "32"),
-        abi::Mipsel => ("little", "mipsel", "32")
-    };
+    let re = Regex::new("([^-]+)-([^-]+)-([^-]+)(-.*)?").unwrap();


While a regex certainly makes sense here, can we avoid pulling in the entire regex crate just for this one use case?

alexcrichton · 2014-08-01T14:35:57Z

src/librustc/driver/config.rs

-        abi::Mipsel => ("little", "mipsel", "32")
-    };
+    let re = Regex::new("([^-]+)-([^-]+)-([^-]+)(-.*)?").unwrap();
+    let matches = re.captures(sess.opts.target_triple.as_slice()).expect("invalid target triple!");


This is dealing with user input so it should probably gracefully fail instead of ICE

alexcrichton · 2014-08-01T14:37:06Z

src/librustc/driver/config.rs

-      abi::Mips => mips::get_target_strs(target_triple, os),
-      abi::Mipsel => mipsel::get_target_strs(target_triple, os)
+    let target = Target::search(sopts.target_triple.as_slice())
+        .expect("Couldn't find target specification, handle this better");


Just a reminder to handle this better

alexcrichton · 2014-08-01T14:38:52Z

src/librustc/metadata/loader.rs

-        }
+    fn dylibname(&self) -> Option<(String, String)> {
+        let t = &self.sess.target.target;
+        Some((t.dll_prefix.clone(), t.dll_suffix.clone()))
    }


This seems like a lossy transformation, why return Option when it's always Some?

alexcrichton · 2014-08-01T14:41:24Z

src/librustc/middle/trans/cabi.rs

+        "x86_64" => cabi_x86_64::compute_abi_info(ccx, atys, rty, ret_def),
+        "arm" => cabi_arm::compute_abi_info(ccx, atys, rty, ret_def),
+        "mips" => cabi_mips::compute_abi_info(ccx, atys, rty, ret_def),
+        a => fail!("unrecognized arch \"{}\" in target descriptor, handle this better", a)


Reminder to handle this better

alexcrichton · 2014-08-01T14:42:36Z

src/librustc/middle/trans/base.rs

-            match fn_ty.abi.for_target(ccx.sess().targ_cfg.os,
-                                       ccx.sess().targ_cfg.arch) {
-                Some(Rust) | Some(RustCall) => {
+            match fn_ty.abi {


How come this stopped calling for_target?

mneumann · 2014-08-01T14:44:09Z

When compiling this on DragonFly I run into these problems:

/home/mneumann/rust/src/liblibc/lib.rs:2943:47: 2943:53 error: unresolved import
 `types::os::arch::c95::c_uint`. Could not find `arch` in `types::os`.
/home/mneumann/rust/src/liblibc/lib.rs:2943             use types::os::arch::c95
::{c_int, c_uint};

          ^~~~~~
/home/mneumann/rust/src/liblibc/lib.rs:2965:17: 2965:44 error: unresolved import
 `types::os::arch::c95::c_int`. Could not find `arch` in `types::os`.
/home/mneumann/rust/src/liblibc/lib.rs:2965             use types::os::arch::c95
::c_int;
                                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mneumann/rust/src/liblibc/lib.rs:2966:17: 2966:49 error: unresolved import
 `types::os::arch::posix88::mode_t`. Could not find `arch` in `types::os`.
/home/mneumann/rust/src/liblibc/lib.rs:2966             use types::os::arch::pos
ix88::mode_t;
                                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mneumann/rust/src/liblibc/lib.rs:3209:17: 3209:44 error: unresolved import
 `types::os::arch::c95::c_int`. Could not find `arch` in `types::os`.
/home/mneumann/rust/src/liblibc/lib.rs:3209             use types::os::arch::c95
::c_int;
                                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mneumann/rust/src/liblibc/lib.rs:150:54: 150:58 error: unresolved import (
maybe you meant `stat::*`?)
/home/mneumann/rust/src/liblibc/lib.rs:150 pub use funcs::posix88::stat_::{chmod
, fstat, mkdir, stat};

                ^~~~
error: aborting due to 115 previous errors
gmake: *** [x86_64-unknown-dragonfly/stage1/lib/rustlib/x86_64-unknown-dragonfly
/lib/stamp.libc] Error 101

Maybe the target_os is incorrectly determined?

alexcrichton · 2014-08-01T14:45:37Z

src/librustc_back/target/apple_base.rs

+
+pub fn target() -> Target {
+    Target {
+        function_sections: false,


false => true ?

Ah nevermind, it would be nice to preserve the comments in the source code to this target specification about why this is false

alexcrichton · 2014-08-01T14:48:01Z

src/librustc_back/target/dragonfly_base.rs

+
+pub fn target() -> Target {
+    Target {
+        function_sections: false,


Was this disabled originally for dragonfly?

alexcrichton · 2014-08-01T14:48:33Z

src/librustc_back/target/freebsd_base.rs

+
+pub fn target() -> Target {
+    Target {
+        function_sections: false,


This should be enabled for freebsd?

alexcrichton · 2014-08-01T14:49:07Z

src/librustc_back/target/freebsd_base.rs

+        executables: true,
+        disable_stack_checking: false,
+        has_rpath: true,
+        relocation_model: "default".to_string(),


Why is this different than "pic"?

alexcrichton · 2014-08-01T14:50:53Z

src/librustc_back/target/i686_unknown_freebsd.rs

+        arch: "x86".to_string(),
+        .. super::freebsd_base::target()
+    }
+}


I don't think split-stacks are supported on 32-bit freebsd, why add this triple?

vadimcn · 2014-10-30T00:49:46Z

Rebased, fixed, squashed: https://github.com/vadimcn/rust/tree/flextarget. Passes tests on Windows.
@cmr: Do you want me to submit it?

See commit message.

aturon · 2014-10-31T20:23:15Z

Revived, yay!

See commit message.

klutzy · 2014-11-01T09:54:19Z

configure

@@ -1100,12 +1123,15 @@ do

    if [ ${do_reconfigure} -ne 0 ]
    then
-        msg "configuring LLVM for $t"
+        # LLVM's configure doesn't recognize the new Windows triples yet
+        $gnu_t=$(to_gnu_triple $t)


- $gnu_t=$(to_gnu_triple $t) + gnu_t=$(to_gnu_triple $t)

Yup, @klutzy is absolutely right! I guess this doesn't break stuff unless LLVM needs to be rebuilt...

See commit message.

vadimcn · 2014-11-02T03:26:09Z

Darn it! What's going on here???

See commit message.

emberian · 2014-11-03T09:27:09Z

I can't replicate the android failure locally. @alexcrichton do you know what API level the emulators are using?

alexcrichton · 2014-11-03T16:30:36Z

Looks like #17448, but I think that we're using api version 19 maybe?

Removes all target-specific knowledge from rustc. Some targets have changed during this, but none of these should be very visible outside of cross-compilation. The changes make our targets more consistent. iX86-unknown-linux-gnu is now only available as i686-unknown-linux-gnu. We used to accept any value of X greater than 1. i686 was released in 1995, and should encompass the bare minimum of what Rust supports on x86 CPUs. The only two windows targets are now i686-pc-windows-gnu and x86_64-pc-windows-gnu. The iOS target has been renamed from arm-apple-ios to arm-apple-darwin. A complete list of the targets we accept now: arm-apple-darwin arm-linux-androideabi arm-unknown-linux-gnueabi arm-unknown-linux-gnueabihf i686-apple-darwin i686-pc-windows-gnu i686-unknown-freebsd i686-unknown-linux-gnu mips-unknown-linux-gnu mipsel-unknown-linux-gnu x86_64-apple-darwin x86_64-unknown-freebsd x86_64-unknown-linux-gnu x86_64-pc-windows-gnu Closes #16093 [breaking-change]

bors · 2014-11-04T10:11:20Z

saw approval from alexcrichton
at https://github.com/cmr/rust/commit/87a753e5ce1885fb94235a227cf6de3a56f2061a

bors · 2014-11-04T10:11:22Z

merging cmr/rust/target-spec = 87a753e into auto

bors · 2014-11-04T10:11:22Z

cmr/rust/target-spec = 87a753e merged ok, testing candidate = cbd0b5d

bors · 2014-11-04T10:16:34Z

See commit message.

bors · 2014-11-04T11:11:18Z

saw approval from alexcrichton
at https://github.com/cmr/rust/commit/61aeab4c9ece6b40e3941665bc9e695922e2ad25

bors · 2014-11-04T11:11:20Z

merging cmr/rust/target-spec = 61aeab4 into auto

bors · 2014-11-04T11:11:21Z

cmr/rust/target-spec = 61aeab4 merged ok, testing candidate = 3a8f4ec

bors · 2014-11-04T13:16:34Z

bors · 2014-11-04T13:21:24Z

fast-forwarding master to auto = 3a8f4ec

See commit message.

vadimcn · 2014-11-04T16:25:05Z

@cmr Congrats! :-)

huonw reviewed Aug 1, 2014
View changes

bvssvni mentioned this pull request Aug 1, 2014

PNaCl support for Rust PistonDevelopers/piston#514

Closed

huonw reviewed Aug 1, 2014
View changes