Skip to content

Fix autodiff debug builds #140030

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.

Sign up for GitHub

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 1 commit into from
Apr 21, 2025
Merged

Fix autodiff debug builds #140030

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion compiler/rustc_monomorphize/src/partitioning.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,9 @@ where
always_export_generics,
);

// We can't differentiate something that got inlined.
// We can't differentiate a function that got inlined.
let autodiff_active = cfg!(llvm_enzyme)
&& matches!(mono_item, MonoItem::Fn(_))
Comment on lines +257 to +259
Copy link
Member

@jieyouxu jieyouxu Apr 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Discussion: this kinda feels backwards to me, I feel like if autodiff is applied to some fn it should inhibit inlining. Is there a small test case (synthetic or not) that can demonstrate this change? Or rather, under what situation can this condition be previously wrong (the change itself looks reasonable)?

Copy link
Member Author

@ZuseZ4 ZuseZ4 Apr 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I add inline(never) in my frontend macro to source and target function. However, those aren't guaranteedon a language level to prevent inlinung, so I additionally check it here in rustc_monomorphize. So yes, autodiff prevents inlining. (Edited a bit, but I think I still miss your point).
https://doc.rust-lang.org/nightly/reference/attributes/codegen.html?highlight=inline
Pretty much every codegen test will fail if you remove these two barriers.

The change btw. is needed since we currently also inspect non fn items like Asm, which triggers a debug assertion, causing bootstrap to fail. But you already mentioned that this looks reasonable to you

Copy link
Member

@jieyouxu jieyouxu Apr 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

But I feel like I probably misunderstand your question?

I moreso meant if there's a smaller example that would fail without this PR, not changing the front-end/middle-end bits, as a regression test.

... But also not building -> building seems like an improvement, yes 😆

Copy link
Member Author

Choose a reason for hiding this comment

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

aah, yes now I get your question. I only mentioned it in the title, not the PR body, but you can find the panic stacktrace in the linked issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

#140064 (comment)

@jieyouxu since you already looked at it, it's a one line change, fixes bootstrap and eliminates the ad overhead, do you mind to rubberstamp it? Then I can drop it from my final enable-aitodiff PR.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, at least this makes it possible to build, and I can't come up with a smaller test...

&& cx
.tcx
.codegen_fn_attrs(mono_item.def_id())
Expand Down
Loading