Add clang enzyme_notypeanalysis and enzyme_ta_norecur attributes - #3072
Add clang enzyme_notypeanalysis and enzyme_ta_norecur attributes#3072markabate wants to merge 4 commits into
Conversation
|
@markabate can you rebase on main, I just landed an infra change [that among other things should fix the use of these within templates]. Also can we come up with a test, e.g. one where an internal function uses a union type or something |
|
Codex: Given Billy’s union/template suggestion and the MFIter construction site, I’d add two lit tests. Background: The AMReX detail worth preserving is that 1. Clang attribute lowering testAdd It should cover:
Sketch: // RUN: if [ %llvmver -ge 12 ]; then %clang++ -std=c++17 -O0 %s \
// RUN: -S -emit-llvm -o - %newLoadClangEnzyme | FileCheck %s; fi
union IterState {
double real;
unsigned long long bits;
};
struct Iter {
IterState state;
// Declaration only, like amrex::MFIter.
[[enzyme::notypeanalysis]]
explicit Iter(int);
};
extern "C" void advance(IterState *)
__attribute__((enzyme_ta_norecur));
extern IterState global_state
__attribute__((enzyme_ta_norecur));
template <typename T>
[[enzyme::notypeanalysis]]
__attribute__((noinline))
static int opaque_union(T input) {
union {
T real;
unsigned long long bits;
} value;
value.real = input;
value.bits = 0;
return static_cast<int>(value.bits);
}
double use(double x) {
Iter iterator(1);
advance(&global_state);
return x + opaque_union(x);
}
// CHECK: @global_state = external global {{.*}}, !enzyme_ta_norecur
// CHECK: declare {{.*}}Iter{{.*}} #[[CTOR:[0-9]+]]
// CHECK: declare void @advance({{.*}}) #[[ADVANCE:[0-9]+]]
// CHECK: define internal {{.*}}opaque_union{{.*}} #[[TEMPLATE:[0-9]+]]
// CHECK-DAG: attributes #[[CTOR]] = { {{.*}}"enzyme_notypeanalysis"{{.*}} }
// CHECK-DAG: attributes #[[ADVANCE]] = { {{.*}}"enzyme_ta_norecur"{{.*}} }
// CHECK-DAG: attributes #[[TEMPLATE]] = { {{.*}}"enzyme_notypeanalysis"{{.*}} }The exact mangled-constructor check should remain loose across LLVM versions. This test also forces the rebased implementation to integrate the new attributes with 2. Direct TypeAnalysis behavior testAdd Use two internal callees whose bodies access the same eight-byte storage as both
Have ; RUN: %opt < %s %newLoadEnzyme -passes="print-type-analysis" \
; RUN: -type-analysis-func=caller -S -o /dev/null | FileCheck %sThe important checks are: ; CHECK-LABEL: caller -
; CHECK-NOT: notype_union -
; CHECK-NOT: norecur_union -That proves the attributes affect TypeAnalysis rather than merely appearing in emitted IR. The same file should contain: @ordinary = internal global i64 0
@ignored = internal global i64 0, !enzyme_ta_norecur !0Propagate a floating-point type through
The unannotated control global specifically catches the missing-braces regression from the review: if Optional diagnostic testA small struct [[enzyme::ta_norecur]] Invalid {};
// expected-warning: attribute only applies to functions and globalsThis ensures invalid placement produces a diagnostic rather than dereferencing a null |
|
@wsmoses Do you have more information on the difference between enzyme_notyeanalysis and enzyme_ta_norecur, to help with the unit tests? My understanding is enzyme_notypeanalysis is only applied to functions and exits from the visitCallBase function here. But visitCallBase only seems to be used with Julia types here. enzyme_ta_norecur seems to exit you from the updateAnalysis loop. |
|
visitCallBase is called by call call instructions [that's just an explicit dispatch]. |
|
and honestly I forgot enzyme_notypeanalysis existed and it should be replaced with the standard enzyme_ta_norecur in all cases |
|
note we can call the c++ attribute enzyme::notypeanalysis or something still |
|
@wsmoses @ax3l I added a unit test and removed the clang attribute I was using to set the enzyme_notypeanalysis internal function attribute - so now there's just one clang attribute called enzyme_notypeanalysis, which sets enzyme_ta_norecur. If you remove __attribute__((enzyme_notypeanalysis)) from the unit test, you get the updateAnalysis error we've been seeing:
I'm still wondering if there's a way Enzyme could gracefully handle situations like this. Could Enzyme leave some types undecided until the activity analysis happens? We can also discuss more at the next design meeting. |
|
yeah there are ways we can try to improve support even without this -- but let's get in one step at a time |
|
also @markabate can you fix the format and remove the old enzyme_notypeanalysis references? |
|
@wsmoses It looks like the enzyme_notypeanalysis annotation was only used in TraceInterface and TraceGenerator: Enzyme/enzyme/Enzyme/TraceInterface.cpp Lines 205 to 217 in 33c495b Enzyme/enzyme/Enzyme/TraceGenerator.cpp Line 173 in 33c495b Do you think this is safe to remove? |
Head branch was pushed to by a user without write access
3 participants
Adding clang attributes for the enzyme_notypeanalysis and enzyme_ta_norecur flags used by Enzyme.
enzyme_notypeanalysis:
https://github.com/search?q=repo%3AEnzymeAD%2FEnzyme+enzyme_notypeanalysis&type=code
enzyme_ta_norecur:
https://github.com/search?q=repo%3AEnzymeAD%2FEnzyme+enzyme_ta_norecur&type=code