Fixes for versioner guard generation
* When calculating the required guard, if a per-arch `introduced`
value is less than the arch min-API, drop the per-arch guard (i.e.
reset the value to 0). This is needed for RISC-V, where we don't
parse the headers with Clang, because the highest APIs we compile
for (e.g. 23, 34) are less than the current RISC-V min API of 10000.
Resetting it to 0 here means we don't need this optimization while
generating an arch-set guard. (i.e. We don't need to calculate
max_min_version. That code should have been calculating a
"min_min_version" anyway.)
* Remove the broken all-supported-archs entry from arch_sets. It has a
few problems:
* It's redundant with the "global availability" code path above,
which is used when the declaration has no per-arch annotations.
* If this code path runs, then we generate two more guard
expressions, for !LP64 and LP64.
* Passing "" to generate_guard is broken for a non-zero version,
and for a zero version, adding an empty string to `expressions`
breaks if the vector has 2 or more expressions.
(I think consolidating per-arch info, e.g. using a single check for
__INTRODUCED_IN_32(40) __INTRODUCED_IN_64(40), is a nice idea, but
it should happen as a natural consequence of removing the
arch-independent "global availability" info in favor of always
tracking it per-arch.)
* Rewrite the arch-set guard generation. Add an optimization so that
the (__ANDROID_API__ >= N) guard for __INTRODUCED_IN_64(N) is still
useful for RISC-V as long as N is small enough. (Currently we're
checking that N is <= 10000.)
This change fixes the "preprocessor" test that run_tests.py runs. The
"slow_preprocessor_idempotence" test is still broken.
Bug: https://github.com/android/ndk/issues/1888
Test: run_tests.py
Change-Id: I3f94357465dbdb2c23fff442a31fb5083de27a97
diff --git a/tools/versioner/src/Preprocessor.cpp b/tools/versioner/src/Preprocessor.cpp
index 14f80d8..47b9017 100644
--- a/tools/versioner/src/Preprocessor.cpp
+++ b/tools/versioner/src/Preprocessor.cpp
@@ -98,7 +98,8 @@
}
for (Arch arch : supported_archs) {
- if (result.arch_availability[arch].introduced <= arch_visibility[arch]) {
+ if (result.arch_availability[arch].introduced <= arch_visibility[arch] ||
+ result.arch_availability[arch].introduced <= arch_min_api[arch]) {
result.arch_availability[arch].introduced = 0;
}
}
@@ -139,9 +140,8 @@
// Logically orred expressions that constitute the macro guard.
std::vector<std::string> expressions;
static const std::vector<std::pair<std::string, std::set<Arch>>> arch_sets = {
- { "", supported_archs },
- { "!defined(__LP64__)", { Arch::arm, Arch::x86 } },
- { "defined(__LP64__)", { Arch::arm64, Arch::riscv64, Arch::x86_64 } },
+ {"!defined(__LP64__)", {Arch::arm, Arch::x86}},
+ {"defined(__LP64__)", {Arch::arm64, Arch::riscv64, Arch::x86_64}},
};
std::map<Arch, std::string> individual_archs = {
{ Arch::arm, "defined(__arm__)" },
@@ -168,6 +168,9 @@
}
if (avail.global_availability.introduced == 0) {
+ // We currently get here for the "__sF" symbol because it's marked __REMOVED_IN(23). This
+ // symbol is the only use of __REMOVED_IN, and it's already guarded manually, so there's no
+ // need to do anything.
fprintf(stderr, "warning: attempted to generate guard with empty availability: %s\n",
to_string(avail).c_str());
return "";
@@ -186,25 +189,35 @@
D(" Checking arch set '%s'\n", arch_expr.c_str());
- int version = avail.arch_availability[*it.second.begin()].introduced;
+ int version = 0;
- // The maximum min_version of the set.
- int max_min_version = 0;
+ // Find the architectures that need to check __ANDROID_API__ and verify that they check against
+ // the same API level.
for (Arch arch : archs) {
- if (arch_min_api[arch] > max_min_version) {
- max_min_version = arch_min_api[arch];
- }
-
- if (avail.arch_availability[arch].introduced != version) {
+ const int arch_version = avail.arch_availability[arch].introduced;
+ if (arch_version == 0) {
+ continue;
+ } else if (version == 0) {
+ version = arch_version;
+ } else if (version != arch_version) {
D(" Skipping arch set, availability for %s doesn't match %s\n",
to_string(*it.second.begin()).c_str(), to_string(arch).c_str());
goto skip;
}
}
- // If all of the archs in the set have a min_api that satifies version, elide the check.
- if (max_min_version >= version) {
- version = 0;
+ // Verify that a non-zero version is acceptable to reuse for other archs with a higher minimum
+ // API, like riscv64. (e.g. It's OK to reuse an (__ANDROID_API__ >= 24) check if the arch's
+ // minimum API is 35.)
+ if (version != 0) {
+ for (Arch arch : archs) {
+ const int arch_version = avail.arch_availability[arch].introduced;
+ if (arch_version == 0 && version > arch_min_api[arch]) {
+ D(" Skipping arch set, availability for %s doesn't match %s\n",
+ to_string(*it.second.begin()).c_str(), to_string(arch).c_str());
+ goto skip;
+ }
+ }
}
expressions.emplace_back(generate_guard(arch_expr, version));
@@ -222,11 +235,7 @@
for (const auto& it : individual_archs) {
const std::string& arch_expr = it.second;
int introduced = avail.arch_availability[it.first].introduced;
- if (introduced == 0) {
- expressions.emplace_back(arch_expr);
- } else {
- expressions.emplace_back(generate_guard(arch_expr, introduced));
- }
+ expressions.emplace_back(generate_guard(arch_expr, introduced));
}
if (expressions.size() == 0) {