Add regression test for graceful error for wrong number of activities

[vayunbiyani]

Fix for: #139590

• Added graceful error message instead of an ICE
• Blessed tests

Edit: one of @ZuseZ4 's commits fixed it. This PR just adds the test case to avoid regressions.

[rustbot]

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[vayunbiyani]

r? @oli-obk
cc: @ZuseZ4

[ZuseZ4]

You'll need to rebase it, since there are some conflicts.
Also, we already have an error message for that which you can use, AutoDiffInvalidNumberActivities so let's not duplicate it. In general, can you please add a testcase which is fixed by this change? Adding your zulip example should work.

[vayunbiyani]

@ZuseZ4 My solution is actually flawed as the error is already detected earlier here but your comment says that // This is not the right signature, but we can continue parsing. Could you please help me understand why we'd want to do so?

[ZuseZ4]

The high-level idea is that if we have multiple functions and multiple autodiff macros per function, then we want to continue parsing all other macros and functions, even if one is incorrect. This way we can report errors in multiple autodiff macros in a single cargo invocation.

To achieve that we only use emit_err instead of emit_fatal_err. Also, if we have to return a dummy function from an incorrect autodiff macro, then we just return any (parsing) body and stop caring if it follows enzyme/autodiff rules. After all we'll never reach our llvm backend if there is any autodiff error in the frontend.

Where possible, we also try to find multiple bugs in a broken autodiff macro, even beyond the first error.
So for example, if someone writes `#[autodiff(df, Reversee, Aactive)], then we want to point out that the Mode is unrecognized (has an extra e) and that the Activity is unrecognized (has an extra a). So we really work hard on not raising a fatal error.

That's also what's motivating the comment. If you look at your example, this is an incorrect application (missing one activity).

#[autodiff(d_ignore, Forward, Dual, Dual)]
fn ignore_one_arg(x: i32, _y: i32) -> i32 {
    x
}

So we don't try to rewrite the function header of d_ignore at all, we just copy the one from ignore_one_arg. This would completely break at the enzyme/llvm backend, but as said we'll never reach it since we raised an error, so that's fine. We just want to finish parsing.

#[rustc_autodiff]
#[inline(never)]
fn ignore_one_arg(x: i32, _y: i32) -> i32 {
    x
}
#[rustc_autodiff(Forward, 1, Dual, Dual)]
#[inline(never)]
fn d_ignore(x: i32, _y: i32) -> i32 {
    unsafe {
        asm!("NOP", options(pure, nomem));
    };
    ::core::hint::black_box(());
    ::core::hint::black_box(<i32>::default())
}

[vayunbiyani]

@rustbot ready

[ZuseZ4]

@vayunbiyani This doesn't include any changes to autodiff files, only to tests. Did you forget to include the fix?

[vayunbiyani]

Yes, it's intentional. One of your commits handled this(I'm not exactly sure at what point tho) leading to no ICEs for the updated case.

[vayunbiyani]

@rustbot ready

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[ZuseZ4]

oh interesting. Sure, then it's fine to just add the test to make sure we don't regress.
Can you undo the lock file change, though? That was likely just some CI noise, rerunning it should fix it.

For the future, just force-pushing the same code with a different hash should trigger a CI re-run if you have unrelated failures. We try to have a reliable CI, but every once in a while random failures happen, just due to the size of the rust CI.

[vayunbiyani]

Hmm, I didn't change anything in the lock file. The commit just reverted to the cargo in the master since it was showing up as a change

[ZuseZ4]

Can you squash your two commits, instead of adding and undoing the changes in the cargo.lock?

[oli-obk]

Please avoid such churn, if you want to insert it here in between and keep the naming scheme use something like f4_1