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

cross compilation improvements #5781

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions crates/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,12 +678,12 @@ pub fn build(
};

let wasm_dev_backend = matches!(code_gen_backend, CodeGenBackend::Wasm);
let force_legacy_linker =
matches.get_one::<String>(FLAG_LINKER).map(|s| s.as_str()) == Some("legacy");

let linking_strategy = if wasm_dev_backend {
LinkingStrategy::Additive
} else if !roc_linker::supported(link_type, &triple)
|| matches.get_one::<String>(FLAG_LINKER).map(|s| s.as_str()) == Some("legacy")
{
} else if !roc_linker::supported(link_type, &triple) || force_legacy_linker {
LinkingStrategy::Legacy
} else {
LinkingStrategy::Surgical
Expand Down
19 changes: 13 additions & 6 deletions crates/compiler/build/src/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,12 +464,13 @@ pub fn rebuild_host(
platform_main_roc.with_file_name(legacy_host_filename(target).unwrap())
};

let env_path = env::var("PATH").unwrap_or_else(|_| "".to_string());
let env_home = env::var("HOME").unwrap_or_else(|_| "".to_string());
let env_cpath = env::var("CPATH").unwrap_or_else(|_| "".to_string());
let env_path = env::var("PATH").unwrap_or_default();
let env_home = env::var("HOME").unwrap_or_default();
let env_cpath = env::var("CPATH").unwrap_or_default();

let builtins_host_tempfile =
roc_bitcode::host_tempfile().expect("failed to write host builtins object to tempfile");
let builtins_host_tempfile = roc_bitcode::host_tempfile(target)
.as_ref()
.expect("failed to write host builtins object to tempfile");

if zig_host_src.exists() {
// Compile host.zig
Expand Down Expand Up @@ -1226,14 +1227,20 @@ fn link_windows(
) -> io::Result<(Child, PathBuf)> {
match link_type {
LinkType::Dylib => {
#[cfg(windows)]
let target = "-native";

#[cfg(not(windows))]
let target = "x86_64-windows-gnu";

let child = zig()
.args(["build-lib"])
.args(input_paths)
.args([
"-lc",
&format!("-femit-bin={}", output_path.to_str().unwrap()),
"-target",
"native",
target,
"--pkg-begin",
"glue",
find_zig_glue_path().to_str().unwrap(),
Expand Down
3 changes: 2 additions & 1 deletion crates/compiler/build/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,8 @@ fn build_loaded_file<'a>(

std::fs::write(app_o_file, &*roc_app_bytes).unwrap();

let builtins_host_tempfile = roc_bitcode::host_tempfile()
let builtins_host_tempfile = roc_bitcode::host_tempfile(target)
.as_ref()
.expect("failed to write host builtins object to tempfile");

let mut inputs = vec![app_o_file.to_str().unwrap()];
Expand Down
1 change: 1 addition & 0 deletions crates/compiler/builtins/bitcode/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ version.workspace = true

[dependencies]
tempfile.workspace = true
target-lexicon.workspace = true

[build-dependencies]
roc_command_utils = { path = "../../../utils/command" }
Expand Down
88 changes: 56 additions & 32 deletions crates/compiler/builtins/bitcode/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ use tempfile::NamedTempFile;
const HOST_WASM: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/builtins-wasm32.o"));
// TODO: in the future, we should use Zig's cross-compilation to generate and store these
// for all targets, so that we can do cross-compilation!
#[cfg(unix)]
const HOST_UNIX: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/builtins-host.o"));
#[cfg(windows)]
const HOST_WINDOWS: &[u8] =
include_bytes!(concat!(env!("OUT_DIR"), "/builtins-windows-x86_64.obj"));

Expand All @@ -21,45 +19,71 @@ pub fn host_wasm_tempfile() -> std::io::Result<NamedTempFile> {
Ok(tempfile)
}

#[cfg(unix)]
fn host_unix_tempfile() -> std::io::Result<NamedTempFile> {
let tempfile = tempfile::Builder::new()
.prefix("host_bitcode")
.suffix(".o")
.rand_bytes(8)
.tempfile()?;
fn current_host_tempfile() -> &'static std::io::Result<NamedTempFile> {
use std::sync::OnceLock;

std::fs::write(tempfile.path(), HOST_UNIX)?;
static TMP: OnceLock<std::io::Result<NamedTempFile>> = OnceLock::new();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should mean that the file is only created once per test run


Ok(tempfile)
}
fn helper() -> std::io::Result<NamedTempFile> {
let tempfile = tempfile::Builder::new()
.prefix("host_bitcode")
.suffix(".o")
.rand_bytes(8)
.tempfile()?;

#[cfg(windows)]
fn host_windows_tempfile() -> std::io::Result<NamedTempFile> {
let tempfile = tempfile::Builder::new()
.prefix("host_bitcode")
.suffix(".obj")
.rand_bytes(8)
.tempfile()?;
std::fs::write(tempfile.path(), HOST_UNIX)?;

std::fs::write(tempfile.path(), HOST_WINDOWS)?;
Ok(tempfile)
}

Ok(tempfile)
TMP.get_or_init(helper)
}

pub fn host_tempfile() -> std::io::Result<NamedTempFile> {
#[cfg(unix)]
{
host_unix_tempfile()
}
fn host_windows_tempfile() -> &'static std::io::Result<NamedTempFile> {
use std::sync::OnceLock;

static TMP: OnceLock<std::io::Result<NamedTempFile>> = OnceLock::new();

#[cfg(windows)]
{
host_windows_tempfile()
fn helper() -> std::io::Result<NamedTempFile> {
let tempfile = tempfile::Builder::new()
.prefix("host_bitcode")
.suffix(".obj")
.rand_bytes(8)
.tempfile()?;

std::fs::write(tempfile.path(), HOST_WINDOWS)?;

Ok(tempfile)
}

#[cfg(not(any(windows, unix)))]
{
unreachable!()
TMP.get_or_init(helper)
}

pub fn host_tempfile(target: &target_lexicon::Triple) -> &'static std::io::Result<NamedTempFile> {
use target_lexicon::Triple;

match target {
Triple {
// architecture: target_lexicon::Architecture::X86_64,
// operating_system: target_lexicon::OperatingSystem::Linux,
Comment on lines +67 to +68
Copy link
Contributor Author

Choose a reason for hiding this comment

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

in practice the "unix" host file is the file compiled for this host. That means that we need to be pretty vague in this pattern to not cause regressions. Eventually we should generate the builtins for more targets, and be more specific here.

binary_format: target_lexicon::BinaryFormat::Elf,
..
} => current_host_tempfile(),

// macho support is incomplete
Triple {
// operating_system: target_lexicon::OperatingSystem::Darwin,
binary_format: target_lexicon::BinaryFormat::Macho,
..
} => current_host_tempfile(),

Triple {
architecture: target_lexicon::Architecture::X86_64,
operating_system: target_lexicon::OperatingSystem::Windows,
binary_format: target_lexicon::BinaryFormat::Coff,
..
} => host_windows_tempfile(),

other => unimplemented!("no host for {other:?} architecture"),
}
}
5 changes: 3 additions & 2 deletions crates/compiler/test_gen/src/helpers/dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,9 @@ pub fn helper(
.expect("failed to build output object");
std::fs::write(&app_o_file, module_out).expect("failed to write object to file");

let builtins_host_tempfile =
roc_bitcode::host_tempfile().expect("failed to write host builtins object to tempfile");
let builtins_host_tempfile = roc_bitcode::host_tempfile(&target)
.as_ref()
.expect("failed to write host builtins object to tempfile");

// TODO make this an environment variable
if false {
Expand Down
54 changes: 28 additions & 26 deletions crates/linker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,33 +32,35 @@ pub enum LinkType {
}

pub fn supported(link_type: LinkType, target: &Triple) -> bool {
if let LinkType::Executable = link_type {
match target {
Triple {
architecture: target_lexicon::Architecture::X86_64,
operating_system: target_lexicon::OperatingSystem::Linux,
binary_format: target_lexicon::BinaryFormat::Elf,
..
} => true,

// macho support is incomplete
Triple {
operating_system: target_lexicon::OperatingSystem::Darwin,
binary_format: target_lexicon::BinaryFormat::Macho,
..
} => false,

Triple {
architecture: target_lexicon::Architecture::X86_64,
operating_system: target_lexicon::OperatingSystem::Windows,
binary_format: target_lexicon::BinaryFormat::Coff,
..
} => true,

_ => false,
match link_type {
LinkType::Executable => {
match target {
Triple {
architecture: target_lexicon::Architecture::X86_64,
operating_system: target_lexicon::OperatingSystem::Linux,
binary_format: target_lexicon::BinaryFormat::Elf,
..
} => true,

// macho support is incomplete
Triple {
operating_system: target_lexicon::OperatingSystem::Darwin,
binary_format: target_lexicon::BinaryFormat::Macho,
..
} => false,

Triple {
architecture: target_lexicon::Architecture::X86_64,
operating_system: target_lexicon::OperatingSystem::Windows,
binary_format: target_lexicon::BinaryFormat::Coff,
..
} => true,

_ => false,
}
}
} else {
false
LinkType::Dylib => false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

linking the dev tests (using zig, today) into a dylib is slow. I think (and fear) that we need to go do this ourselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe it's not so bad? it's just a fancy copy https://github.com/gimli-rs/object/blob/master/crates/examples/src/bin/pecopy.rs

fancy because we need to join two files, the test code and the builtins.

LinkType::None => false,
}
}

Expand Down
Loading