Skip to content

refactor(autodiff): Simplify Autodiff Handling of rlib Dependencies#153379

Merged
rust-bors[bot] merged 2 commits intorust-lang:mainfrom
TKanX:refactor/149164-simplify-autodiff-rlib
Mar 11, 2026
Merged

refactor(autodiff): Simplify Autodiff Handling of rlib Dependencies#153379
rust-bors[bot] merged 2 commits intorust-lang:mainfrom
TKanX:refactor/149164-simplify-autodiff-rlib

Conversation

@TKanX
Copy link
Contributor

@TKanX TKanX commented Mar 4, 2026

Summary:

Resolves the two FIXMEs left in #149033, per @bjorn3 guidance in the discussion.

Closes #149164

r? @ZuseZ4
cc @bjorn3

@rustbot
Copy link
Collaborator

rustbot commented Mar 4, 2026

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_llvm/src/builder/autodiff.rs

cc @ZuseZ4

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. F-autodiff `#![feature(autodiff)]` labels Mar 4, 2026
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 4, 2026
@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@TKanX TKanX marked this pull request as draft March 4, 2026 02:41
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2026
@TKanX TKanX force-pushed the refactor/149164-simplify-autodiff-rlib branch from 08fc7e9 to 671a24e Compare March 4, 2026 02:51
@TKanX TKanX marked this pull request as ready for review March 4, 2026 03:00
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 4, 2026
@rustbot

This comment has been minimized.

};

let fn_ptr_ty =
Ty::new_fn_ptr(tcx, fn_source.ty(tcx, TypingEnv::fully_monomorphized()).fn_sig(tcx));
Copy link
Member

Choose a reason for hiding this comment

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

The other half of the fix is changing the rustc_autodiff intrinsic to accept a function pointer rather than function item.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great! It can be even less code. Love reducing code.

@TKanX TKanX marked this pull request as draft March 4, 2026 11:35
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2026
@TKanX TKanX force-pushed the refactor/149164-simplify-autodiff-rlib branch from 671a24e to cd9aa24 Compare March 4, 2026 11:38
@TKanX TKanX marked this pull request as ready for review March 4, 2026 11:43
@rustbot
Copy link
Collaborator

rustbot commented Mar 4, 2026

Some changes occurred in compiler/rustc_builtin_macros/src/autodiff.rs

cc @ZuseZ4

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 4, 2026
@rustbot

This comment has been minimized.

@TKanX TKanX requested a review from bjorn3 March 4, 2026 11:45
TKanX added 2 commits March 4, 2026 03:53
@TKanX TKanX force-pushed the refactor/149164-simplify-autodiff-rlib branch from cd9aa24 to 8a3d0f4 Compare March 4, 2026 11:54
@ZuseZ4
Copy link
Member

ZuseZ4 commented Mar 10, 2026

Thanks! I verified that tests still pass locally with this. I'm slightly disappointed that this makes writing the intrinsic by hand a little more annoying, but we only do that very rarely when prototyping some frontend changes. Re-reading Bjorn's comment, I guess that also was to be expected. The benefits of being able to completely remove autodiff handling from rustc_mir_transform and rustc_monomorphize heavily outweigh the slightly increased intrinsic complexity, so

@bors r+

cc @saethlin

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 10, 2026

📌 Commit 8a3d0f4 has been approved by ZuseZ4

It is now in the queue for this repository.

@rust-bors rust-bors bot 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 Mar 10, 2026
@rust-bors

This comment has been minimized.

@rust-bors rust-bors bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 11, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 11, 2026

☀️ Test successful - CI
Approved by: ZuseZ4
Duration: 3h 8m
Pushing d1c7945 to main...

@rust-bors rust-bors bot merged commit d1c7945 into rust-lang:main Mar 11, 2026
12 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 11, 2026
@TKanX TKanX deleted the refactor/149164-simplify-autodiff-rlib branch March 11, 2026 05:12
@github-actions
Copy link
Contributor

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 b49ecc9 (parent) -> d1c7945 (this PR)

Test differences

Show 16 test diffs

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

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard d1c79458b5d13bd0179d7dbafd5ca4ea9ae3e6aa --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. dist-aarch64-apple: 1h 36m -> 2h 41m (+67.1%)
  2. x86_64-gnu-debug: 1h 44m -> 2h 5m (+20.1%)
  3. pr-check-1: 27m 46s -> 33m 18s (+19.9%)
  4. dist-x86_64-apple: 1h 53m -> 2h 14m (+18.6%)
  5. x86_64-rust-for-linux: 45m 10s -> 53m 20s (+18.1%)
  6. arm-android: 1h 33m -> 1h 47m (+15.1%)
  7. dist-aarch64-llvm-mingw: 1h 44m -> 1h 28m (-14.9%)
  8. i686-gnu-2: 1h 30m -> 1h 42m (+13.2%)
  9. x86_64-gnu-aux: 2h 9m -> 2h 24m (+11.4%)
  10. i686-gnu-1: 2h 6m -> 2h 20m (+10.8%)
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

Finished benchmarking commit (d1c7945): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.1% [0.1%, 0.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -1.7%, secondary 1.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
7.6% [7.6%, 7.6%] 1
Improvements ✅
(primary)
-1.7% [-1.7%, -1.7%] 1
Improvements ✅
(secondary)
-2.2% [-3.5%, -1.0%] 2
All ❌✅ (primary) -1.7% [-1.7%, -1.7%] 1

Cycles

Results (primary 1.7%, secondary -3.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.7% [1.7%, 1.7%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.5% [-3.5%, -3.5%] 1
All ❌✅ (primary) 1.7% [1.7%, 1.7%] 1

Binary size

Results (primary -0.0%, secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.0% [-0.1%, -0.0%] 28
Improvements ✅
(secondary)
-0.1% [-0.1%, -0.0%] 19
All ❌✅ (primary) -0.0% [-0.1%, -0.0%] 28

Bootstrap: 479.93s -> 479.3s (-0.13%)
Artifact size: 396.95 MiB -> 396.94 MiB (-0.00%)

#[rustc_autodiff(Forward, 1, Dual, Const, Dual)]
pub fn df1(x: &[f64], bx_0: &[f64], y: f64) -> (f64, f64) {
::core::intrinsics::autodiff(f1::<>, df1::<>, (x, bx_0, y))
::core::intrinsics::autodiff(f1::<> as fn(_: &[f64], _: f64) -> f64,
Copy link
Member

Choose a reason for hiding this comment

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

f1::<> as _ may also work. That should allow getting rid of most code you added to compiler/rustc_builtin_macros/src/autodiff.rs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need a follow-up PR?

Copy link
Member

Choose a reason for hiding this comment

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

I tried locally updating the primal_fn_ptr to
let primal_fn_ptr = ecx.expr(span, ast::ExprKind::Cast(primal_path_expr, ecx.ty(span, TyKind::Infer)));:

45	#[rustc_autodiff(Forward, 1, Dual, Const, Const)]
46	pub fn df2(x: &[f64], bx_0: &[f64], y: f64) -> f64 {
-	    ::core::intrinsics::autodiff(f2::<> as fn(_: &[f64], _: f64) -> f64,
-	        df2::<>, (x, bx_0, y))
+	    ::core::intrinsics::autodiff(f2::<> as _, df2::<>, (x, bx_0, y))
49	}

But unfortunately, type inference fails in all cases then:

------rustc stderr------------------------------
error[E0282]: type annotations needed
  --> /home/manuel/prog/rust/tests/codegen-llvm/autodiff/generic.rs:16:1
   |
16 | #[autodiff_reverse(d_square, Duplicated, Active)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type

Copy link
Member

Choose a reason for hiding this comment

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

That is unfortunate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. F-autodiff `#![feature(autodiff)]` merged-by-bors This PR was explicitly merged by bors. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simplify autodiff handling of dependencies (rlib)

6 participants