Split `autodiff` into `autodiff_forward` and `autodiff_reverse` #140697

Sa4dUs · 2025-05-06T07:25:52Z

This PR splits #[autodiff] macro so #[autodiff(df, Reverse, args)] would become #[autodiff_reverse(df, args)] and #[autodiff(df, Forward, args)] would become #[autodiff_forwad(df, args)].

Pending tasks:

Fix cannot find a built-in macro with name error.
Update the tests to reflect the new macro structure.
Review the implementation for better code.

ZuseZ4 · 2025-05-06T13:58:17Z

I'd almost bet it has to do with bootstrapping, see e.g. https://github.com/rust-lang/rust/pull/129458/files#diff-ea30f57e78f3b861b784ae315b3ac31358b7cea2aa14a668084ad412de12e586
It will be removed by other people later after landing, that's probably why you didn't see it anymore when just looking at the current code for it. Ping me if that doesn't solve the issue, then I can try to play around with it locally.

jieyouxu · 2025-05-07T10:06:07Z

I don't remember exactly, can built-in attributes be namespaced now? E.g. #[autodiff::forward()] and #[autodiff::reverse()]. That way, only one "top-level" name would cause macro ambiguities from the built-in attribute (autodiff), instead of two.

ZuseZ4 · 2025-05-07T10:13:55Z

I think :: looks much prettier than _. Our attribute is called rustc_autodiff and not autodiff, so I don't think there will be ambiguities either way.
@jdonszelmann is probably the right person to ask about parsing. (see the question above)

jieyouxu · 2025-05-07T11:18:57Z

Right sorry, I keep forgetting there's two

Pending fix. ``` error: cannot find a built-in macro with name `autodiff_forward` --> library\core\src\macros\mod.rs:1542:5 | 1542 | / pub macro autodiff_forward($item:item) { 1543 | | /* compiler built-in */ 1544 | | } | |_____^ error: cannot find a built-in macro with name `autodiff_reverse` --> library\core\src\macros\mod.rs:1549:5 | 1549 | / pub macro autodiff_reverse($item:item) { 1550 | | /* compiler built-in */ 1551 | | } | |_____^ error: could not compile `core` (lib) due to 2 previous errors ```

UI tests are pending, will depend on error messages change.

…des (`Forward`, `Reverse`) Since the mode is no longer part of `meta_item`, we must insert it manually (otherwise macro expansion with `#[rustc_autodiff]` won't work). This can be revised later if a more structured representation becomes necessary (using enums, annotated structs, etc). Some tests are currently failing. I'll address them next.

ZuseZ4 · 2025-05-10T17:44:42Z

Try build/x86_64-unknown-linux-gnu/llvm/bin/clang-format compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp -i (Adjusted for all the cpp files you touched).

Sa4dUs · 2025-05-10T18:09:58Z

I'd say the tidy error it's because compiler/rustc_builtin_macros/messages.ftl has the message builtin_macros_autodiff_mode, but it's not being used. I still need to fix a bug when parsing the arguments (I'm assuming it's a silly mistake with an index or array boundary), and once everything works properly, I'll take care of errors and such, which I assume will need to be adjusted somehow for the new macros.

bors · 2025-05-13T14:06:30Z

☔ The latest upstream changes (presumably #140887) made this pull request unmergeable. Please resolve the merge conflicts.