Skip to content

Commit 51a79e3

Browse files
committed
addressing final feedback
1 parent c5adbf2 commit 51a79e3

File tree

4 files changed

+38
-5
lines changed

4 files changed

+38
-5
lines changed

Diff for: compiler/rustc_builtin_macros/src/autodiff.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ mod llvm_enzyme {
591591
if x.mode.is_fwd() {
592592
// Fwd mode is easy. If the return activity is Const, we support arbitrary types.
593593
// Otherwise, we only support a scalar, a pair of scalars, or an array of scalars.
594+
// We checked that (on a best-effort base) in the preceding gen_enzyme_decl function.
594595
// In all three cases, we can return `std::hint::black_box(<T>::default())`.
595596
if x.ret_activity == DiffActivity::Const {
596597
// Here we call the primal function, since our dummy function has the same return
@@ -625,15 +626,16 @@ mod llvm_enzyme {
625626
exprs = ecx.expr_tuple(new_decl_span, exprs2);
626627
}
627628
_ => {
629+
// Interestingly, even the `-> ArbitraryType` case
630+
// ends up getting matched and handled correctly above,
631+
// so we don't have to handle any other case for now.
628632
panic!("Unsupported return type: {:?}", d_ret_ty);
629633
}
630634
}
631635
}
632636
exprs = ecx.expr_call(new_decl_span, bb_call_expr, thin_vec![exprs]);
633637
} else {
634-
// The return type of our dummy function is the same as the return type of our primal.
635-
// In that case, we can just use the primal call as last instruction.
636-
panic!("Unsupported mode: {:?}", x.mode);
638+
unreachable!("Unsupported mode: {:?}", x.mode);
637639
}
638640

639641
body.stmts.push(ecx.stmt_expr(exprs));

Diff for: tests/codegen/autodiffv.rs

+7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
//@ compile-flags: -Zautodiff=Enable -C opt-level=3 -Clto=fat
22
//@ no-prefer-dynamic
33
//@ needs-enzyme
4+
//
5+
// In Enzyme, we test against a large range of LLVM versions (5+) and don't have overly many
6+
// breakages. One benefit is that we match the IR generated by Enzyme only after running it
7+
// through LLVM's O3 pipeline, which will remove most of the noise.
8+
// However, our integration test could also be affected by changes in how rustc lowers MIR into
9+
// LLVM-IR, which could cause additional noise and thus breakages. If that's the case, we should
10+
// reduce this test to only match the first lines and the ret instructions.
411

512
#![feature(autodiff)]
613

Diff for: tests/ui/autodiff/autodiff_illegal.rs

+7
Original file line numberDiff line numberDiff line change
@@ -177,4 +177,11 @@ fn f21(x: f32) -> f32 {
177177
unimplemented!()
178178
}
179179

180+
struct DoesNotImplDefault;
181+
#[autodiff(df22, Forward, Dual)]
182+
pub fn f22() -> DoesNotImplDefault {
183+
//~^^ ERROR the function or associated item `default` exists for tuple `(DoesNotImplDefault, DoesNotImplDefault)`, but its trait bounds were not satisfied
184+
unimplemented!()
185+
}
186+
180187
fn main() {}

Diff for: tests/ui/autodiff/autodiff_illegal.stderr

+19-2
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,24 @@ error[E0433]: failed to resolve: use of undeclared type `F64Trans`
151151
LL | #[autodiff(df18, Reverse, Active, Active)]
152152
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use of undeclared type `F64Trans`
153153

154-
error: aborting due to 22 previous errors
154+
error[E0599]: the function or associated item `default` exists for tuple `(DoesNotImplDefault, DoesNotImplDefault)`, but its trait bounds were not satisfied
155+
--> $DIR/autodiff_illegal.rs:181:1
156+
|
157+
LL | struct DoesNotImplDefault;
158+
| ------------------------- doesn't satisfy `DoesNotImplDefault: Default`
159+
LL | #[autodiff(df22, Forward, Dual)]
160+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function or associated item cannot be called on `(DoesNotImplDefault, DoesNotImplDefault)` due to unsatisfied trait bounds
161+
|
162+
= note: the following trait bounds were not satisfied:
163+
`DoesNotImplDefault: Default`
164+
which is required by `(DoesNotImplDefault, DoesNotImplDefault): Default`
165+
help: consider annotating `DoesNotImplDefault` with `#[derive(Default)]`
166+
|
167+
LL + #[derive(Default)]
168+
LL | struct DoesNotImplDefault;
169+
|
170+
171+
error: aborting due to 23 previous errors
155172

156-
Some errors have detailed explanations: E0428, E0433, E0658.
173+
Some errors have detailed explanations: E0428, E0433, E0599, E0658.
157174
For more information about an error, try `rustc --explain E0428`.

0 commit comments

Comments
 (0)