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

Autodiff Upstreaming - rustc_codegen_llvm changes #178

Closed
wants to merge 5,139 commits into from
Closed

Conversation

ZuseZ4
Copy link
Member

@ZuseZ4 ZuseZ4 commented Sep 7, 2024

Now that the autodiff/Enzyme backend is merged, this is an upstream PR for the rustc_codegen_llvm changes.
It also includes small changes to three files under compiler/rustc_ast, which overlap with my frontend PR.
Here I only include minimal definitions of structs and enums to be able to build this backend code.
The same goes for minimal changes to compiler/rustc_codegen_ssa, the majority of changes there will be in another PR, once either this or the frontend gets merged.

We currently have 68 files left to merge, 19 in the frontend PR, 21 (+3 from the frontend) in this PR, and then ~30 in the middle-end.

To already highlight the things which reviewers might want to discuss:

  1. enzyme_ffi.rs: I do have a fallback module to make sure that we don't link rustc against Enzyme when we build rustc without autodiff support.

  2. add_panic_msg_to_global was a pain to write and I currently can't even use it. Enzyme writes gradients into shadow memory. Pass in one float scalar? We'll allocate and return an extra float telling you how this float affected the output. Pass in a slice of floats? We'll let you allocate the vector and pass in a mutable reference to a float slice, we'll then write the gradient into that slice. It should be at least as large as your original slice, so we check that and panic if not. Currently we panic silently, but I already generate a nicer panic message with this function. I just don't know how to print it to the user. yet. I discussed this with a few rustc devs and the best we could come up with (for now), was to look for mangled panic calls in the IR and pick one, which works surprising reliably. If someone knows a good way to clean this up and print the panic message I'm all in, otherwise I can remove the code that writes the nicer panic message and keep the silent panic, since it's enough for soundness. Especially since this PR is already a bit larger.

  3. SanitizeHWAddress: When differentiating C++, Enzyme can use TBAA to "understand" enums/unions, but for Rust we don't have this information. LLVM might to speculative loads which (without TBAA) confuse Enzyme, so we disable those with this attribute. This attribute is only set during the first opt run before Enzyme differentiates code. We then remove it again once we are done with autodiff and run the opt pipeline a second time. Since enums are everywhere in Rust, support for them is crucial, but if this looks too cursed I can remove these ~100 lines and keep them in my fork for now, we can then discuss them separately to make this PR simpler?

  4. Duplicated llvm-opt runs: Differentiating already optimized code (and being able to do additional optimizations on the fly, e.g. for GPU code) is the reason why Enzyme is so fast, so the compile time is acceptable for autodiff users: https://enzyme.mit.edu/talks/Publications/ (There are also algorithmic issues in Enzyme core which are more serious than running opt twice).

  5. I assume that if we merge these minimal cg_ssa changes here already, I also need to fix the other backends (GCC and cliff) to have dummy implementations, correct?

  6. I'm happy to split this PR up further if reviewers have recommendations on how to.

For the full implementation, see: rust-lang#129175

Tracking:

@ZuseZ4 ZuseZ4 force-pushed the master branch 3 times, most recently from 4f0389e to 012062a Compare October 28, 2024 02:32
@ZuseZ4 ZuseZ4 force-pushed the enzyme-cg-llvm branch 12 times, most recently from 469aaf0 to 79a3e2b Compare November 24, 2024 04:09
notriddle and others added 11 commits December 26, 2024 15:46
This is a solution to the `std::sync::poison` linking problem,
and, in general, makes intra-doc links shorter and clearer.
…kang

[macro_metavar_expr_concat] Fix rust-lang#128346

Fix rust-lang#128346
Fix rust-lang#131393

The syntax is invalid in both issues so I guess that theoretically the compiler should have aborted early.

This PR tries to fix a local problem but let me know if there are better options.

cc `@petrochenkov` if you are interested
Add `into_array` conversion destructors for `Box`, `Rc`, and `Arc`.

Tracking issue: rust-lang#133508

This PR adds the `into_array` destructor for `alloc::boxed::Box<[T]>`, `alloc::rc::Rc<[T]>`, and `alloc::sync::Arc<[T]>`.

Note that this PR assumes the initial proposal of these functions returning `Option`. It is still an open question whether this should instead be `Result`. We can, however, easily change this in a follow-up PR with the necessary consensus.
Document collection `From` and `FromIterator` impls that drop duplicate keys.

This behavior is worth documenting because there are other plausible alternatives, such as panicking when a duplicate is encountered, and it reminds the programmer to consider whether they should, for example, coalesce duplicate keys first.

Followup to rust-lang#89869.
Fix forgetting to save statx availability on success

Looks like we forgot to save the statx state on success which means the first failure (common when checking if a file exists) will always require spending an invalid statx to confirm the failure is real.

r? `@thomcc`
Use scoped threads in `std::sync::Barrier` examples

This removes boilerplate around `Arc`s and makes the code more clear.
…ark-Simulacrum

Update Code Example for `Iterator::rposition`

Added an additional assertion to the example to show the behavior of `iter.next_back` after using `iter.rposition`.
…5.0, r=Mark-Simulacrum

unwinding: bump version to fix naked_asm on Xous

With rust-lang#80608 the `unwinding` crate no longer builds. The upstream crate has been updated to build by manually adding directives to the naked_asm stream.

Bump the dependency in Rust to get this newer version. This fixes the build for Xous, and closes rust-lang#134403.
ehuss and others added 25 commits December 31, 2024 08:50
…inop-indent, r=compiler-errors"

This reverts commit 3628783, reversing
changes made to 31026b7.
…elds have defaults

```
error: `Default` impl doesn't use the declared default field values
  --> $DIR/manual-default-impl-could-be-derived.rs:28:1
   |
LL | / impl Default for B {
LL | |     fn default() -> Self {
LL | |         B {
LL | |             x: s(),
   | |                --- this field has a default value
LL | |             y: 0,
   | |                - this field has a default value
...  |
LL | | }
   | |_^
   |
help: to avoid divergence in behavior between `Struct { .. }` and `<Struct as Default>::default()`, derive the `Default`
   |
LL ~ #[derive(Default)] struct B {
   |
```

Note that above the structured suggestion also includes completely removing the manual `impl`, but the rendering doesn't.
Tidy up bigint multiplication methods

This tidies up the library version of the bigint multiplication methods after the addition of the intrinsics in rust-lang#133663. It follows [this summary](rust-lang#85532 (comment)) of what's desired for these methods.

Note that, if `2H = N`, then `uH::MAX * uH::MAX + uH::MAX + uH::MAX` is `uN::MAX`, and that we can effectively add two "carry" values without overflowing.

For ease of terminology, the "low-order" or "least significant" or "wrapping" half of multiplication will be called the low part, and the "high-order" or "most significant" or "overflowing" half of multiplication will be called the high part. In all cases, the return convention is `(low, high)` and left unchanged by this PR, to be litigated later.

## API Changes

The original API:

```rust
impl uN {
    // computes self * rhs
    pub const fn widening_mul(self, rhs: uN) -> (uN, uN);

    // computes self * rhs + carry
    pub const fn carrying_mul(self, rhs: uN, carry: uN) -> (uN, uN);
}
```

The added API:

```rust
impl uN {
    // computes self * rhs + carry1 + carry2
    pub const fn carrying2_mul(self, rhs: uN, carry: uN, add: uN) -> (uN, uN);
}
impl iN {
    // note that the low part is unsigned
    pub const fn widening_mul(self, rhs: iN) -> (uN, iN);
    pub const fn carrying_mul(self, rhs: iN, carry: iN) -> (uN, iN);
    pub const fn carrying_mul_add(self, rhs: iN, carry: iN, add: iN) -> (uN, iN);
}
```

Additionally, a naive implementation has been added for `u128` and `i128` since there are no double-wide types for those. Eventually, an intrinsic will be added to make these more efficient, but rather than doing this all at once, the library changes are added first.

## Justifications for API

The unsigned parts are done to ensure consistency with overflowing addition: for a two's complement integer, you want to have unsigned overflow semantics for all parts of the integer except the highest one. This is because overflow for unsigned integers happens on the highest bit (from `MAX` to zero), whereas overflow for signed integers happens on the second highest bit (from `MAX` to `MIN`). Since the sign information only matters in the highest part, we use unsigned overflow for everything but that part.

There is still discussion on the merits of signed bigint *addition* methods, since getting the behaviour right is very subtle, but at least for signed bigint *multiplication*, the sign of the operands does make a difference. So, it feels appropriate that at least until we've nailed down the final API, there should be an option to do signed versions of these methods.

Additionally, while it's unclear whether we need all three versions of bigint multiplication (widening, carrying-1, and carrying-2), since it's possible to have up to two carries without overflow, there should at least be a method to allow that. We could potentially only offer the carry-2 method and expect that adding zero carries afterword will optimise correctly, but again, this can be litigated before stabilisation.

## Note on documentation

While a lot of care was put into the documentation for the `widening_mul` and `carrying_mul` methods on unsigned integers, I have not taken this same care for `carrying_mul_add` or the signed versions. While I have updated the doc tests to be more appropriate, there will likely be many documentation changes done before stabilisation.

## Note on tests

Alongside this change, I've added several tests to ensure that these methods work as expected. These are alongside the codegen tests for the intrinsics.
Update cargo

5 commits in c86f4b3a1b153218e6e50861214b0b4b4e695f23..d73d2caf9e41a39daf2a8d6ce60ec80bf354d2a7
2024-12-24 17:49:48 +0000 to 2024-12-31 20:51:21 +0000
- fix(package): warn if symlinks checked out as plain text files (rust-lang/cargo#14994)
- test: track caller for `.crate` file publish verification (rust-lang/cargo#14992)
- test: relax panic output assertion (rust-lang/cargo#14989)
- test: relax `bad_crate_type` to only match error message prefix (rust-lang/cargo#14990)
- refactor(package): split `cargo_package` to modules (rust-lang/cargo#14982)

r? ghost
…d-2, r=Mark-Simulacrum

Add more mailmap entries

If you have been pinged: I'm adding a mailmap entry for you to keep thanks.rust-lang.org working properly.

**I have picked the canonical address mostly arbitrarily. If you want a different one as the canonical address, please tell me here**.

<details>
<summary>more details on the change</summary>

builds on top of rust-lang#132471, this containing the less obvious changes that add new canonical email addresses instead of just adding new current ones.

The people updated in this commit have contributed under different email
addresses than the ones they have used in rust-lang/team.
rust-lang/thanks#53 will use team data for thanks reviewers, which requires this to be in
sync.
Therefore, I have updated many of the people that I've noticed being
duplicated after the change.

</details>

I'm adding a novel entry for you: ``@apiraino`` ``@KodrAus`` ``@cramertj`` ``@sunfishcode`` ``@Eh2406`` ``@skade`` ``@huonw`` ``@jsha`` ``@shepmaster`` ``@workingjubilee`` ``@rbtcollins``  ``@nrc`` ``@fitzgen`` ``@sophiajt`` ``@tmiasko`` ``@notriddle`` ``@TimNN`` ``@WaffleLapkin``
…r=estebank

borrowck diagnostics: make `add_move_error_suggestions` use the HIR rather than `SourceMap`

This PR aims to fix rust-lang#132806 by rewriting `add_move_error_suggestions`[^1]. Previously, it manually scanned the source text to find a leading `&`, which isn't always going to produce a correct result (see: that issue). Admittedly, the HIR visitor in this PR introduces a lot of boilerplate, but hopefully the logic at its core isn't too complicated (I go over it in the comments). I also tried a simpler version that didn't use a HIR visitor and suggested adding `ref` always, but the `&ref x` suggestions really didn't look good. As a bonus for the added complexity though, it's now able to produce nice `&`-removing suggestions in more cases.

I tried to do this such that it avoids edition-dependent checks and its suggestions can be applied together with those from the match ergonomics 2024 migration lint. I haven't added tests for that since the details of match ergonomics 2024 are still being sorted out, but I can try if desired once that's finalized.

[^1]: In brief, it fires on patterns where users try to bind by-value in such a way that moves out of a reference to a non-Copy type (including slice references with non-copy elements). The suggestions are to change the binding's mode to be by-reference, either by removing[^2] an enclosing `&`/`&mut` or adding `ref` to the binding.

[^2]: Incidentally, I find the terminology of "consider removing the borrow" a bit confusing for a suggestion to remove a `&` pattern in order to make bindings borrow rather than move. I'm not sure what a good, concise way to explain that would be though, and that should go in a separate PR anyway.
…=fmease

Add GUI test for item info elements color

Fixes rust-lang#98341.

r? ``@fmease``
Print how to rebless Python formatting in tidy

Suggested [here](rust-lang#134964 (comment)).

r? ``@Noratrieb``
chore: fix typos

I hope my typo corrections will help you. Thank you for your work.
add .mailmap entry for myself

Seeing rust-lang#132474 in the bors queue I decided to check myself and noticed I was listed four times.
This fixed that by cannonicalizing all entries to use my username and the github no-reply email address, rather than some combination of name or username and different email addresses.
…compiler-errors

Revert rust-lang#119515 single line where clause style guide

This did not get implemented for the style edition in 2024, so this PR removes it from the documentation.
See tracking issue rust-lang#132381.

This can be added back in the next edition if it gets implemented. I'm a little unclear on what the style team intends for how future changes are documented. For example, the current style-guide documented behavior that rustfmt does not support. I'm not sure who the audience for this document is, or how this is intended to stay in sync with rustfmt. For example, if I read this and assume this is how it is supposed to work, and then rustfmt breaks that, it seems like that is confusing. Similarly, if I'm staying on an older edition, this documentation would be incorrect for my crate.

Perhaps changes like this could be "teed-up" in a PR, but not merged until the edition is stabilized (similar to how the reference works)? And include notes for parts that are edition-specific (so if I am using an older edition, I can see that something is different). In general, I'm a little confused on how this is intended to work.

Reverts:

- rust-lang#119515
…, r=compiler-errors

Revert style guide rhs break

This reverts rust-lang#132369 and rust-lang#119838. The style-guide change for indentation of rhs was not implemented in time for the 2024 style edition.
See tracking issue rust-lang#132380.

cc rust-lang#134974 for the other style guide change in 2024.

r? ``@compiler-errors``
Rollup of 8 pull requests

Successful merges:

 - rust-lang#132474 (Add more mailmap entries)
 - rust-lang#133486 (borrowck diagnostics: make `add_move_error_suggestions` use the HIR rather than `SourceMap`)
 - rust-lang#134861 (Add GUI test for item info elements color)
 - rust-lang#134968 (Print how to rebless Python formatting in tidy)
 - rust-lang#134971 (chore: fix typos)
 - rust-lang#134972 (add .mailmap entry for myself)
 - rust-lang#134974 (Revert rust-lang#119515 single line where clause style guide)
 - rust-lang#134975 (Revert style guide rhs break)

r? `@ghost`
`@rustbot` modify labels: rollup
Update books

## rust-lang/book

13 commits in ad2011d3bcad9f152d034faf7635c22506839d58..04d06dfe541607e6419f3d028c3f9b245f3be4d9
2024-12-20 22:44:11 UTC to 2024-12-16 18:18:21 UTC

- Update section name (rust-lang/book#4175)
- Update text to reflect the change from '&str' to 'char' in example (rust-lang/book#4173)
- Update figure number in text and file name (rust-lang/book#4172)
- Fix chapter number (rust-lang/book#4171)
- Delete unused reference (rust-lang/book#4170)
- Remove orphaned half-sentence (rust-lang/book#4169)
- Fix chapter number (rust-lang/book#4168)
- A better phrasing for generic methods. (rust-lang/book#3428)
- Fix minor grammatical error (rust-lang/book#4098)
- Update appendix-06-translation.md to add another Chinese translation. (rust-lang/book#3608)
- Change Korean translation repo to newer (rust-lang/book#3625)
- Clarify/improve readability in Ch. 02 discussion of `Result` variants (rust-lang/book#4167)
- Ch. 20: conclude §01 with a reference to the Rustonomicon (rust-lang/book#4166)

## rust-lang/nomicon

1 commits in 97e84a38c94bf9362b11284c20b2cb4adaa1e868..7ef05b9777c94836bc92f50f23e6e00981521a89
2024-12-30 10:38:10 UTC to 2024-12-30 10:38:10 UTC

- Fix URL fragment to MutexGuard's negative impl of Send (rust-lang/nomicon#472)

## rust-lang/reference

3 commits in 9f41bc11342d46544ae0732caf14ec0bcaf27376..acd6794e712d5e2ef6f5c84fb95688d32a69b816
2024-12-18 23:04:30 +0000 to 2024-12-30 22:12:57 +0000

- Revert "`coverage` attribute" (rust-lang/reference#1706)
- Document Rust 2024 match ergonomics reservations (rust-lang/reference#1702)
- Add documentation for `#[diagnostic::do_not_recommend]` (rust-lang/reference#1663)

## rust-lang/edition-guide

5 commits in bc4ce51e1d4dacb9350a92e95f6159a42de2f8c6..d56e0f3a0656b7702ca466d4b191e16c28262b82
2024-12-18 05:34:59 +0000 to 2024-12-31 20:04:52 +0000

- 2024: Document rustfmt overflow_delimited_expr (rust-lang/edition-guide#352)
- 2024: Document rustfmt fixes (rust-lang/edition-guide#351)
- Update the transitioning steps (rust-lang/edition-guide#350)
- Revert "2024: Assignment operator RHS indentation" (rust-lang/edition-guide#343)
- Revert "2024: Add chapter on single-line `where` clauses" (rust-lang/edition-guide#344)

## rust-lang/rust-by-example

1 commits in 76406337f4131253443aea0ed7e7f451b464117c..093397535b48ae13ec76bc526b7e6eb8c096a85c
2024-12-18 17:29:56 UTC to 2024-12-18 17:29:56 UTC

- Mention that you're not allowed to partially move Drop types (rust-lang/rust-by-example#1902)

## rust-lang/rustc-dev-guide

18 commits in 7f7ba48f04abc2ad25e52f30b5e2bffa286b019f..ad93c5f1c49f2aeb45f7a4954017b1e607df9f5e
2024-12-30 09:22:22 UTC to 2024-12-17 17:00:38 UTC

- Opt into, rather than out of, linkcheck (rust-lang/rustc-dev-guide#2180)
- Remove stale implementation details of coverage instrumentation (rust-lang/rustc-dev-guide#2179)
- Remove properly tracked config file from .gitignore & add support for skipping of link-checking (rust-lang/rustc-dev-guide#2023)
- Add a couple of linkcheck exceptions: (rust-lang/rustc-dev-guide#2120)
- Add missing link for [Node] (rust-lang/rustc-dev-guide#2177)
- Fix 403 received for HEAD request (rust-lang/rustc-dev-guide#2176)
- Start using mdbook-linkcheck2 (rust-lang/rustc-dev-guide#2103)
- Document `x test --no-capture` (rust-lang/rustc-dev-guide#2174)
- Remove the `-test` suffix from normalize directives (rust-lang/rustc-dev-guide#2172)
- Rework the driver docs (rust-lang/rustc-dev-guide#2162)
- Document `forbid-output` for UI tests (rust-lang/rustc-dev-guide#2171)
- completions: Zsh is now supported (rust-lang/rustc-dev-guide#2173)
- region-outlives propagation (rust-lang/rustc-dev-guide#2169)
- compiletest: Document the `--debugger` flag (rust-lang/rustc-dev-guide#2170)
- document the public ci dashboard (rust-lang/rustc-dev-guide#2167)
- Fix trivial typo of "query-fied" (rust-lang/rustc-dev-guide#2165)
- Fix some typos (rust-lang/rustc-dev-guide#2166)
- Add suggestion for `--keep-stage 0` (rust-lang/rustc-dev-guide#2164)
…t, r=estebank

Remove allowing static_mut_refs lint
…bank

E0277: suggest dereferencing function arguments in more cases

This unifies and generalizes some of the logic in `TypeErrCtxt::suggest_dereferences` so that it will suggest dereferencing arguments to function/method calls in order to satisfy trait bounds in more cases.

Previously it would only fire on reference types, and it had two separate cases (one specifically to get through custom `Deref` impls when passing by-reference, and one specifically to catch rust-lang#87437). I've based the new checks loosely on what's done for `E0308` in `FnCtxt::suggest_deref_or_ref`: it will suggest dereferences to satisfy trait bounds whenever the referent is `Copy`, is boxed (& so can be moved out of the boxes), or is being passed by reference.

This doesn't make the suggestion fire in contexts other than function arguments or binary operators (which are in a separate case that this doesn't touch), and doesn't make it suggest a combination of `&`-removal and dereferences. Those would require a bit more restructuring, so I figured just doing this would be a decent first step.

Closes rust-lang#90997
…p-message, r=estebank

add suggestion for wrongly ordered format parameters

Add suggestion for wrongly ordered format parameters like `?#`.

Supersedes rust-lang#131004
Fix rust-lang#129966
…=estebank

Some small nits to the borrowck suggestions for mutating a map through index

1. Suggesting users to either use `.insert` or `.get_mut` (which do totally different things) can be a bit of a footgun, so let's make that a bit more nuanced.
2. I find the suggestion of `.get_mut(|val| { *val = whatever; })` to be a bit awkward. I changed this to be an if-let instead.
3. Fix a bug which was suppressing the structured suggestion for some mutations via the index operator on `HashMap`/`BTreeMap`.

r? estebank or reassign
bootstrap: Overhaul and simplify the `tool_check_step!` macro

Main changes:

- Pull most of `run` out of the macro and into a regular helper function
- Reduce the number of redundant/unnecessary macro arguments
- Switch to struct-like syntax so that optional arguments are clearer, and so that rustfmt is happy

~~The one “functional” change is that the `-check.stamp` files now get their name from the final path segment, instead of the struct name; in practice this means that they now contain more hyphens in some cases. As far as I'm aware, the exact filename doesn't matter so this should be fine.~~ (that change has been removed from this PR)
…iler-errors

Provide structured suggestion for `impl Default` of type where all fields have defaults

```
error: `Default` impl doesn't use the declared default field values
  --> $DIR/manual-default-impl-could-be-derived.rs:28:1
   |
LL | / impl Default for B {
LL | |     fn default() -> Self {
LL | |         B {
LL | |             x: s(),
   | |                --- this field has a default value
LL | |             y: 0,
   | |                - this field has a default value
...  |
LL | | }
   | |_^
   |
help: to avoid divergence in behavior between `Struct { .. }` and `<Struct as Default>::default()`, derive the `Default`
   |
LL ~ #[derive(Default)] struct B {
   |
```

Note that above the structured suggestion also includes completely removing the manual `impl`, but the rendering doesn't.
Rollup of 6 pull requests

Successful merges:

 - rust-lang#131439 (Remove allowing static_mut_refs lint)
 - rust-lang#133292 (E0277: suggest dereferencing function arguments in more cases)
 - rust-lang#134877 (add suggestion for wrongly ordered format parameters)
 - rust-lang#134945 (Some small nits to the borrowck suggestions for mutating a map through index)
 - rust-lang#134950 (bootstrap: Overhaul and simplify the `tool_check_step!` macro)
 - rust-lang#134979 (Provide structured suggestion for `impl Default` of type where all fields have defaults)

r? `@ghost`
`@rustbot` modify labels: rollup
…merjake

char to_digit: avoid unnecessary casts to u64

Hello,
in the `char::to_digit` method there are a few `as u64` casts that are not strictly necessary.
I assume that the reason behind these casts is to avoid possible overflows in the `+ 10` add.

This PR removes the aforementioned casts, avoiding the overflow issue by slightly modifying the ASCII letter to int mapping.

Thanks,
Happy new year.
Run Python formatting check in tidy on CI

I don't think that there's a reason why we should ignore Python formatting on CI, when we already check Python lints and C++ formatting.

r? `@onur-ozkan`
@ZuseZ4
Copy link
Member Author

ZuseZ4 commented Jan 6, 2025

merged upstream into rust-lang/rust.

@ZuseZ4 ZuseZ4 closed this Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.