Skip to content

Rollup of 8 pull requests #140256

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

Merged
merged 21 commits into from
Apr 24, 2025
Merged

Rollup of 8 pull requests #140256

merged 21 commits into from
Apr 24, 2025

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

beetrees and others added 21 commits March 20, 2025 02:01
This commit adjusts feature implication of the RISC-V ISA for better
feature detection from the user perspective.

The main rule is:
If the feature A is a functional superset of the feature B (A ⊃ B),
A is to imply B, even if this implication is not on the manual.

Such implications (not directly referred in the ISA manual) are commented
as "A ⊃ B" which means "A is a (functional) superset of B".

1.  Zbc → Zbkc (add as a superset)
    The Zbkc extension is a subset of the Zbc extension
    (Zbc - "clmulr" instruction == Zbkc)
2.  Zkr → (nothing) (remove dependency to Zicsr)
    Implication to the Zicsr extension is removed because (although nearly
    harmless), the Zkr extension (or the "seed" CSR section) defines its own
    subset of the Zicsr extension.
3.  Zvbb → Zvkb (comment as a superset)
    This implication was already there but not denoted as a functional
    superset.  This commit adds the comment.
4.  Zvfh → Zvfhmin (comment as a superset)
    This is similar to the case above (Zvbb → Zvkb).
5.  Zvfh → Zve32f (add implication per the ISA specification)
    This dependency is on the ISA manual but was missing (due to the fact
    that Zvfh indirectly implies Zve32f on the current implementation
    through Zvfh → Zvfhmin, which is a functional relation).
    This commit ensures that this is *also* ISA-compliant in the
    source code level (there's no functional changes though).
6.  Zvknhb → Zvknha (add as a superset)
    The Zvknhb extension (SHA-256 / SHA-512) is a functional superset of
    the Zvknha extension (SHA-256 only).
I missed this in rust-lang#139868. Its `mod` declaration was removed, but the
contents were not moved.
…ions of 'f16', 'f32', 'f64', and 'f128' into 'const fn' items;
It is no longer needed after a recent refactoring.
Suggest {to,from}_ne_bytes for transmutations between arrays and integers, etc

implements rust-lang#136067

Rust has helper methods for many kinds of safe transmutes, for example integer<->bytes. This is a lint against using transmute for these cases.

```rs
fn bytes_at_home(x: [u8; 4]) -> u32 {
   transmute(x)
}

// other examples
transmute::<[u8; 2], u16>();
transmute::<[u8; 8], f64>();
transmute::<u32, [u8; 4]>();
transmute::<char, u32>();
transmute::<u32, char>();
```
It would be handy to suggest `u32::from_ne_bytes(x)`.
This is implemented for `[u8; _]` -> `{float int}`

This also implements the cases:
`fXX` <-> `uXX` = `{from_bits, to_bits}`
`uXX` -> `iXX` via `cast_unsigned` and `cast_signed`
{`char` -> `u32`, `bool` -> `n8`} via `from`
`u32` -> `char` via `from_u32_unchecked` (note: notes `from_u32().unwrap()`) (contested)
`u8` -> `bool` via `==` (debatable)

---
try-job: aarch64-gnu
try-job: test-various
…li-obk

Add `#[repr(u128)]`/`#[repr(i128)]` enums to `improper_ctypes_definitions`

This makes them warn whenever a plain `u128`/`i128` would. If the lang team decides to merge rust-lang#137306 then this can be reverted.

Tracking issue: rust-lang#56071
Autodiff flags

Interestingly, it seems that some other projects have conflicts with exactly the same LLVM optimization passes as autodiff.
At least `LLVMRustOptimize` has exactly the flags that we need to disable problematic opt passes.

This PR enables us to compile code where users differentiate two identical functions in the same module. This has been especially common in test cases, but it's not impossible to encounter in the wild.

It also enables two new flags for testing/debugging. I consider writing an MCP to upgrade PrintPasses to be a standalone -Z flag, since it is *not* the same as `-Z print-llvm-passes`, which IMHO gives less useful output. A discussion can be found here: [#t-compiler/llvm > Print llvm passes. @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/187780-t-compiler.2Fllvm/topic/Print.20llvm.20passes.2E/near/511533038)

Finally, it improves `PrintModBefore` and `PrintModAfter`. They used to work reliable, but now we just schedule enzyme as part of an existing ModulePassManager (MPM). Since Enzyme is last in the MPM scheduling, PrintModBefore became very inaccurate. It used to print the input module, which we gave to the Enzyme and was great to create llvm-ir reproducer. However, lately the MPM would run the whole `default<O3>` pipeline, which heavily modifies the llvm module, before we pass it to Enzyme. That made it impossible to use the flag to create llvm-ir reproducers for Enzyme bugs. We now schedule a PrintModule pass just before Enzyme, solving this problem.

Based on the PrintPass output, it also _seems_ like changing `registerEnzymeAndPassPipeline(PB, true);` to `registerEnzymeAndPassPipeline(PB, false);` has no effect. In theory, the bool should tell Enzyme to schedule some helpful passes in the PassBuilder. However, since it doesn't do anything and I'm not 100% sure anymore on whether we really need it, I'll just disable it for now and postpone investigations.

r? ``@oli-obk``

closes rust-lang#139471

Tracking:

- rust-lang#124509
… r=Amanieu

rustc_target: Adjust RISC-V feature implication

This commit adjusts feature implication of the RISC-V ISA for better feature detection from the user perspective.

The main rule is:

*   If the feature `A` is a functional superset of the feature `B` (`A ⊃ B`),
    `A` is to imply `B`, even if this implication is not on the manual.

Such implications (not directly written in the ISA manual) are commented as `A ⊃ B`
which means "`A` is a (functional) superset of `B`".

1.  `Zbc` → `Zbkc` (add as a superset)
    The `Zbkc` extension is a subset of the `Zbc` extension (`Zbc` minus `clmulr` instruction).
2.  `Zkr` → (nothing) (remove dependency to `Zicsr`)
    Implication to the `Zicsr` extension is removed because (although nearly harmless), the `Zkr` extension (or the `seed` CSR section) defines its own subset of the `Zicsr` extension (guaranteed to work against the `seed` CSR which needs read/write access).
3.  `Zvbb` → `Zvkb` (comment as a superset)
    This implication was already there but not denoted as a functional superset.  This commit adds the comment.
4.  `Zvfh` → `Zvfhmin` (comment as a superset)
    This is similar to the case above (`Zvbb` → `Zvkb`).
5.  `Zvfh` → `Zve32f` (add implication per the ISA specification)
    This dependency is on the ISA manual but was missing (due to the fact that `Zvfh` indirectly implies `Zve32f` on the current implementation through `Zvfh` → `Zvfhmin` which is a functional relation). This commit ensures that this is *also* ISA-compliant in the source code level (there's no functional changes though).
6.  `Zvknhb` → `Zvknha` (add as a superset)
    The `Zvknhb` extension (SHA-256 / SHA-512) is a functional superset of the `Zvknha` extension (SHA-256 only).
Move zkVM constants into `sys::env_consts`

I missed this in rust-lang#139868. Its `mod` declaration was removed, but the contents were not moved.

r? joboet
fix MAX_EXP and MIN_EXP docs

As pointed out in rust-lang#88734, the docs for these constants are wrong.

r? ``@tgross35``
…r=RalfJung

Make algebraic functions into `const fn` items.

Tracking issue: rust-lang#136469

This PR makes the algebraic intrinsics and the unstable, algebraic functions of `f16`, `f32`, `f64`, and `f128` into `const fn` items:

```rust
impl f16 {
    pub const fn algebraic_add(self, rhs: f16) -> f16;
    pub const fn algebraic_sub(self, rhs: f16) -> f16;
    pub const fn algebraic_mul(self, rhs: f16) -> f16;
    pub const fn algebraic_div(self, rhs: f16) -> f16;
    pub const fn algebraic_rem(self, rhs: f16) -> f16;
}

impl f32 {
    pub const fn algebraic_add(self, rhs: f32) -> f32;
    pub const fn algebraic_sub(self, rhs: f32) -> f32;
    pub const fn algebraic_mul(self, rhs: f32) -> f32;
    pub const fn algebraic_div(self, rhs: f32) -> f32;
    pub const fn algebraic_rem(self, rhs: f32) -> f32;
}

impl f64 {
    pub const fn algebraic_add(self, rhs: f64) -> f64;
    pub const fn algebraic_sub(self, rhs: f64) -> f64;
    pub const fn algebraic_mul(self, rhs: f64) -> f64;
    pub const fn algebraic_div(self, rhs: f64) -> f64;
    pub const fn algebraic_rem(self, rhs: f64) -> f64;
}

impl f128 {
    pub const fn algebraic_add(self, rhs: f128) -> f128;
    pub const fn algebraic_sub(self, rhs: f128) -> f128;
    pub const fn algebraic_mul(self, rhs: f128) -> f128;
    pub const fn algebraic_div(self, rhs: f128) -> f128;
    pub const fn algebraic_rem(self, rhs: f128) -> f128;
}

// core::intrinsics

pub const fn fadd_algebraic<T: Copy>(a: T, b: T) -> T;
pub const fn fsub_algebraic<T: Copy>(a: T, b: T) -> T;
pub const fn fmul_algebraic<T: Copy>(a: T, b: T) -> T;
pub const fn fdiv_algebraic<T: Copy>(a: T, b: T) -> T;
pub const fn frem_algebraic<T: Copy>(a: T, b: T) -> T;
```

This PR does not preserve the initial behaviour of these functions yielding non-deterministic output under Miri; it is most likely desired to reimplement this behaviour at some point.
…git-config, r=jieyouxu

Remove git repository from git config

It is no longer needed after rust-lang#138591. We could even remove the `nightly_branch` field, but it still has one usage.

r? ``@jieyouxu``
@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc F-autodiff `#![feature(autodiff)]` S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue. labels Apr 24, 2025
@rustbot rustbot added the rollup A PR which is a rollup label Apr 24, 2025
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Collaborator

bors commented Apr 24, 2025

📌 Commit a016552 has been approved by matthiaskrgr

It is now in the queue for this repository.

@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 Apr 24, 2025
@bors
Copy link
Collaborator

bors commented Apr 24, 2025

⌛ Testing commit a016552 with merge d7ea436...

@bors
Copy link
Collaborator

bors commented Apr 24, 2025

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing d7ea436 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 24, 2025
@bors bors merged commit d7ea436 into rust-lang:master Apr 24, 2025
7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 24, 2025
Copy link

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 3c877f6 (parent) -> d7ea436 (this PR)

Test differences

Show 1211 test diffs

Stage 1

  • [codegen] tests/codegen/autodiff/identical_fnc.rs: [missing] -> ignore (ignored when LLVM Enzyme is disabled) (J2)
  • [ui] tests/ui/transmute/unnecessary-transmutation.rs: [missing] -> pass (J2)

Stage 2

  • [ui] tests/ui/transmute/unnecessary-transmutation.rs: [missing] -> pass (J0)
  • [codegen] tests/codegen/autodiff/identical_fnc.rs: [missing] -> ignore (ignored when LLVM Enzyme is disabled) (J1)

Additionally, 1207 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard d7ea436a02d5de4033fcf7fd4eb8ed965d0f574c --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. x86_64-apple-2: 6857.0s -> 4757.0s (-30.6%)
  2. dist-x86_64-apple: 7461.6s -> 9270.3s (24.2%)
  3. dist-apple-various: 7675.3s -> 5967.1s (-22.3%)
  4. dist-x86_64-linux: 5671.9s -> 4945.4s (-12.8%)
  5. x86_64-msvc-ext2: 5650.8s -> 5988.1s (6.0%)
  6. dist-aarch64-apple: 5191.8s -> 5490.5s (5.8%)
  7. x86_64-gnu-llvm-20-3: 6757.4s -> 7107.2s (5.2%)
  8. x86_64-gnu: 6558.8s -> 6221.6s (-5.1%)
  9. i686-gnu-nopt-2: 6731.7s -> 6396.2s (-5.0%)
  10. x86_64-apple-1: 7613.3s -> 7986.4s (4.9%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#136083 Suggest {to,from}_ne_bytes for transmutations between array… 1e087fdb58f5c41f49669285e0e215278ad0f83a (link)
#138282 Add #[repr(u128)]/#[repr(i128)] enums to `improper_ctyp… 9ad8f44b0cfcc20d5f4dc7b7c7a2161a037933c8 (link)
#139700 Autodiff flags 55db1e079726789b3c591c81c69c5bd83308f959 (link)
#140139 rustc_target: Adjust RISC-V feature implication e3c5feda1ae0a241a08247640b5bac27bbf41bb5 (link)
#140141 Move zkVM constants into sys::env_consts ba1958074482da4c1f74b196e907f3e6e1077d94 (link)
#140150 fix MAX_EXP and MIN_EXP docs 1bc4d17b5a4355dd47d4ca8ebe05b7f5f01defcd (link)
#140172 Make algebraic functions into const fn items. 55e1af99a149789b9383e6752a7721948210e7d1 (link)
#140191 Remove git repository from git config f7bf72af7f05b3047d28bdfe05c422444c8f58bb (link)

previous master: 3c877f6a47

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d7ea436): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
0.2% [0.2%, 0.2%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.6%, -0.2%] 23
All ❌✅ (primary) 0.2% [0.2%, 0.2%] 1

Max RSS (memory usage)

Results (primary 0.5%, secondary 0.1%)

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)
1.0% [0.4%, 2.1%] 4
Regressions ❌
(secondary)
6.6% [5.3%, 7.8%] 2
Improvements ✅
(primary)
-1.3% [-1.3%, -1.3%] 1
Improvements ✅
(secondary)
-2.6% [-3.0%, -1.8%] 5
All ❌✅ (primary) 0.5% [-1.3%, 2.1%] 5

Cycles

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

Binary size

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

Bootstrap: 777.873s -> 777.966s (0.01%)
Artifact size: 365.12 MiB -> 365.11 MiB (-0.00%)

@rustbot rustbot added the perf-regression Performance regression. label Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc F-autodiff `#![feature(autodiff)]` merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.