improve compiler&tool documenting and re-enable cranelift on CI #117574

onur-ozkan · 2023-11-04T12:10:04Z

First commit addresses the linking issue with compiler crates. Second one ensures that compiler crates are linked correctly (with later commits we added this check for tools as well), allowing us to detect these hard-to-catch bugs on CI. Following three commits cherry-picked from #117328 to re-enable the Cranelift backend on CI.

More info: #117430

cc @bjorn3 @RalfJung

rustbot · 2023-11-04T12:10:12Z

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

Signed-off-by: onur-ozkan <[email protected]>

Either generate it with the actual codegen backend dylib or omit it entirely when the cranelift backend is disabled for this build.

self.number_of_times_dry_runs_have_caused_issues = 1;

Signed-off-by: onur-ozkan <[email protected]>

bjorn3 · 2023-11-04T12:51:22Z

Thanks a lot for working on this!

Could you add the following patch? I found this unintended change while I was (unsuccessfully) trying to figure out the doc issue too.

Author: bjorn3 <17426603 [email protected]>
Date:   Sat Nov 4 12:47:05 2023  0000

    Don't disable inline asm usage in compiler-builtins when the cranelift backend is enabled
    
    This was a leftover from when inline asm wasn't universally supported by
    the cranelift backend yet. It would cause the optimized inline asm
    intrinsics to be avoided for the standard library distributed with
    rustup now that we build the cranelift backend by default on nightly for
    several tier 1 targets.

diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs
index 1eed534150b..8ab0ef61c50 100644
--- a/src/bootstrap/src/core/build_steps/compile.rs
    b/src/bootstrap/src/core/build_steps/compile.rs
@@ -413,11  413,6 @@ pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, car
 
     let mut features = String::new();
 
-    // Cranelift doesn't support `asm`.
-    if stage != 0 && builder.config.default_codegen_backend().unwrap_or_default() == "cranelift" {
-        features  = " compiler-builtins-no-asm";
-    }
-
     if builder.no_std(target) == Some(true) {
         features  = " compiler-builtins-mem";
         if !target.starts_with("bpf") {

onur-ozkan · 2023-11-04T13:01:38Z

Thanks a lot for working on this!

Could you add the following patch? I found this unintended change while I was (unsuccessfully) trying to figure out the doc issue too.

Author: bjorn3 <17426603 [email protected]>
Date:   Sat Nov 4 12:47:05 2023  0000

    Don't disable inline asm usage in compiler-builtins when the cranelift backend is enabled
    
    This was a leftover from when inline asm wasn't universally supported by
    the cranelift backend yet. It would cause the optimized inline asm
    intrinsics to be avoided for the standard library distributed with
    rustup now that we build the cranelift backend by default on nightly for
    several tier 1 targets.

diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs
index 1eed534150b..8ab0ef61c50 100644
--- a/src/bootstrap/src/core/build_steps/compile.rs
    b/src/bootstrap/src/core/build_steps/compile.rs
@@ -413,11  413,6 @@ pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, car
 
     let mut features = String::new();
 
-    // Cranelift doesn't support `asm`.
-    if stage != 0 && builder.config.default_codegen_backend().unwrap_or_default() == "cranelift" {
-        features  = " compiler-builtins-no-asm";
-    }
-
     if builder.no_std(target) == Some(true) {
         features  = " compiler-builtins-mem";
         if !target.starts_with("bpf") {

It gives "fatal: empty ident name (for <>) not allowed" when I try to git am on the patch. Is it ok for you if I do git apply on it (it will show my name instead of yours on the commit)?

bjorn3 · 2023-11-04T13:31:27Z

You could try git commit --amend --author "bjorn3 <17426603 [email protected]>" to change the author of the commit. If that doesn't work, plain git apply is fine with me.

…t backend is enabled This was a leftover from when inline asm wasn't universally supported by the cranelift backend yet. It would cause the optimized inline asm intrinsics to be avoided for the standard library distributed with rustup now that we build the cranelift backend by default on nightly for several tier 1 targets.

onur-ozkan · 2023-11-04T14:23:39Z

Seems like rustdoc and rustdoc_json_types docs are also missing. Let's block the PR until I fix them.

@rustbot blocked

Mark-Simulacrum · 2023-11-04T16:28:11Z

In general r=me, but happy to see this build docs in CI (via a try build, likely with a change like this to re-enable docs: 43f592f) before we land it. Happy to re-review with the extra fix you mention (but not strictly needed if similar changes).

onur-ozkan · 2023-11-04T16:41:39Z

@bors try

bors · 2023-11-04T16:42:50Z

⌛ Trying commit a348b00 with merge 4d1b721...

…g, r=<try> re-enable cranelift on CI First commit addresses the linking issue with compiler crates. Second one ensures that compiler crates are linked correctly, allowing us to detect these hard-to-catch bugs on CI. The remaining three commits cherry-picked from rust-lang#117328 to re-enable the Cranelift backend on CI. More info: rust-lang#117430 cc `@bjorn3` `@RalfJung`

bors · 2023-11-04T18:02:50Z

💔 Test failed - checks-actions

onur-ozkan · 2023-11-04T18:14:46Z

@bors try

bors · 2023-11-05T09:31:09Z

⌛ Trying commit a3fe394 with merge 83e9fc5...

…g, r=<try> re-enable cranelift on CI First commit addresses the linking issue with compiler crates. Second one ensures that compiler crates are linked correctly, allowing us to detect these hard-to-catch bugs on CI. Following three commits cherry-picked from rust-lang#117328 to re-enable the Cranelift backend on CI. More info: rust-lang#117430 cc `@bjorn3` `@RalfJung`

bors · 2023-11-05T09:52:54Z

💔 Test failed - checks-actions

onur-ozkan · 2023-11-05T09:57:11Z

@bors try

bors · 2023-11-05T09:58:20Z

⌛ Trying commit 32946d2 with merge dc8c831...

…g, r=<try> re-enable cranelift on CI First commit addresses the linking issue with compiler crates. Second one ensures that compiler crates are linked correctly, allowing us to detect these hard-to-catch bugs on CI. Following three commits cherry-picked from rust-lang#117328 to re-enable the Cranelift backend on CI. More info: rust-lang#117430 cc `@bjorn3` `@RalfJung`

bors · 2023-11-05T11:41:00Z

☀️ Try build successful - checks-actions
Build commit: dc8c831 (dc8c8311ce31a9e8e92f63c098be23b53f2bfc21)

onur-ozkan · 2023-11-05T11:51:26Z

Here the try build that includes compiler docs: https://ci-artifacts.rust-lang.org/rustc-builds/dc8c8311ce31a9e8e92f63c098be23b53f2bfc21/rustc-docs-nightly-x86_64-unknown-linux-gnu.tar.xz. However, it's missing rustdoc pages. I will trigger bors for another try build (I can't debug this build locally, working with a laptop atm) to see if this issue gets resolved. If not, I will continue working on it tonight or early tomorrow on my desktop (so I can debug things faster).

EDIT: It seems the last attempt fixed rustdoc problem!

onur-ozkan · 2023-11-05T11:52:32Z

@bors try

bors · 2023-11-05T11:53:42Z

⌛ Trying commit 43bc2dd with merge d4862cd...

…g, r=<try> re-enable cranelift on CI First commit addresses the linking issue with compiler crates. Second one ensures that compiler crates are linked correctly, allowing us to detect these hard-to-catch bugs on CI. Following three commits cherry-picked from rust-lang#117328 to re-enable the Cranelift backend on CI. More info: rust-lang#117430 cc `@bjorn3` `@RalfJung`

bors · 2023-11-05T13:34:48Z

☀️ Try build successful - checks-actions
Build commit: d4862cd (d4862cdec9eae562e5318823f9a7374b10d6acb6)

onur-ozkan · 2023-11-05T13:57:45Z

It's ready now! Here is the build https://ci-artifacts.rust-lang.org/rustc-builds/d4862cdec9eae562e5318823f9a7374b10d6acb6/rustc-docs-nightly-x86_64-unknown-linux-gnu.tar.xz

Signed-off-by: onur-ozkan <[email protected]>

onur-ozkan · 2023-11-05T14:10:53Z

Happy to re-review with the extra fix you mention (but not strictly needed if similar changes).

No strict changes have been applied. Also, we are able to catch missing-doc problem for compiler and tool crates (like here https://github.com/rust-lang-ci/rust/actions/runs/6760207883/job/18373700086#step:25:28933), so it should be fine to r.

@bors r=Mark-Simulacrum rollup=never

bors · 2023-11-05T14:10:56Z

📌 Commit b0df821 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

bors · 2023-11-05T15:41:38Z

⌛ Testing commit b0df821 with merge 6c7de31...

bors · 2023-11-05T17:40:30Z

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 6c7de31 to master...

rust-timer · 2023-11-06T09:25:09Z

Finished benchmarking commit (6c7de31): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.7%	[1.6%, 3.7%]	5
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.0%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.6%	[-1.1%, 3.7%]	7

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.5%, 0.6%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.5%	[-0.6%, -0.5%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-0.6%, 0.6%]	4

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 635.747s -> 638.861s (0.49%)
Artifact size: 304.50 MiB -> 304.51 MiB (0.00%)

rustbot assigned Mark-Simulacrum Nov 4, 2023

onur-ozkan and others added 5 commits November 4, 2023 15:11

Ensure compiler crate paths are generated before linking

5e2de47

Signed-off-by: onur-ozkan <[email protected]>

add sanity check for compiler crate docs

fda009c

Signed-off-by: onur-ozkan <[email protected]>

pass CODEGEN_BACKENDS to docker

d82fdb6

Ensure the rustc-codegen-cranelift-preview component is never empty

a9913eb

Either generate it with the actual codegen backend dylib or omit it entirely when the cranelift backend is disabled for this build.

handle dry runs in dist::CodegenBackend

451778b

self.number_of_times_dry_runs_have_caused_issues = 1;

onur-ozkan force-pushed the fix-compiler-crate-linking branch from 91d297f to 451778b Compare November 4, 2023 12:12

ensure doc directory is not empty

55b4945

Signed-off-by: onur-ozkan <[email protected]>

onur-ozkan force-pushed the fix-compiler-crate-linking branch from 49e5ef5 to 55b4945 Compare November 4, 2023 12:46

onur-ozkan force-pushed the fix-compiler-crate-linking branch from 2501bb6 to 55b4945 Compare November 4, 2023 12:56

rustbot added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2023

onur-ozkan removed the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Nov 4, 2023

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 4, 2023

onur-ozkan force-pushed the fix-compiler-crate-linking branch from a3fe394 to 32946d2 Compare November 5, 2023 09:56

onur-ozkan force-pushed the fix-compiler-crate-linking branch from 32946d2 to 43bc2dd Compare November 5, 2023 11:51

onur-ozkan changed the title ~~re-enable cranelift on CI~~ improve compiler&tool documenting and re-enable cranelift on CI Nov 5, 2023

bootstrap: improve linking of tool docs

b0df821

Signed-off-by: onur-ozkan <[email protected]>

onur-ozkan force-pushed the fix-compiler-crate-linking branch from 43bc2dd to b0df821 Compare November 5, 2023 14:02

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 5, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 5, 2023

bors merged commit 6c7de31 into rust-lang:master Nov 5, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Nov 5, 2023

github-actions bot mentioned this pull request Nov 6, 2023

Automated pull from upstream master ferrocene/ferrocene#84

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve compiler&tool documenting and re-enable cranelift on CI #117574

improve compiler&tool documenting and re-enable cranelift on CI #117574

onur-ozkan commented Nov 4, 2023 •

edited

Loading

rustbot commented Nov 4, 2023

bjorn3 commented Nov 4, 2023

onur-ozkan commented Nov 4, 2023 •

edited

Loading

bjorn3 commented Nov 4, 2023

onur-ozkan commented Nov 4, 2023

Mark-Simulacrum commented Nov 4, 2023

onur-ozkan commented Nov 4, 2023

bors commented Nov 4, 2023

bors commented Nov 4, 2023

onur-ozkan commented Nov 4, 2023

bors commented Nov 5, 2023

bors commented Nov 5, 2023

onur-ozkan commented Nov 5, 2023

bors commented Nov 5, 2023

bors commented Nov 5, 2023

onur-ozkan commented Nov 5, 2023 •

edited

Loading

onur-ozkan commented Nov 5, 2023

bors commented Nov 5, 2023

bors commented Nov 5, 2023

onur-ozkan commented Nov 5, 2023

onur-ozkan commented Nov 5, 2023

bors commented Nov 5, 2023

bors commented Nov 5, 2023

bors commented Nov 5, 2023

rust-timer commented Nov 6, 2023

improve compiler&tool documenting and re-enable cranelift on CI #117574

improve compiler&tool documenting and re-enable cranelift on CI #117574

Conversation

onur-ozkan commented Nov 4, 2023 • edited Loading

rustbot commented Nov 4, 2023

bjorn3 commented Nov 4, 2023

onur-ozkan commented Nov 4, 2023 • edited Loading

bjorn3 commented Nov 4, 2023

onur-ozkan commented Nov 4, 2023

Mark-Simulacrum commented Nov 4, 2023

onur-ozkan commented Nov 4, 2023

bors commented Nov 4, 2023

bors commented Nov 4, 2023

onur-ozkan commented Nov 4, 2023

bors commented Nov 5, 2023

bors commented Nov 5, 2023

onur-ozkan commented Nov 5, 2023

bors commented Nov 5, 2023

bors commented Nov 5, 2023

onur-ozkan commented Nov 5, 2023 • edited Loading

onur-ozkan commented Nov 5, 2023

bors commented Nov 5, 2023

bors commented Nov 5, 2023

onur-ozkan commented Nov 5, 2023

onur-ozkan commented Nov 5, 2023

bors commented Nov 5, 2023

bors commented Nov 5, 2023

bors commented Nov 5, 2023

rust-timer commented Nov 6, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

onur-ozkan commented Nov 4, 2023 •

edited

Loading

onur-ozkan commented Nov 4, 2023 •

edited

Loading

onur-ozkan commented Nov 5, 2023 •

edited

Loading