[vm/compiler] Reduce number of subtype comparisons in AOT.
Previously when building subtype class id ranges for a class in AOT, we
looped over the entire class table, even though in most cases, the
number of actual subclasses and implementors of the class is a small
subset of all loaded classes.
Instead, use the same hierarchy information in both JIT and AOT, only
falling back to traversing the entire class table in cases where the
hierarchy information is missing.
Additional changes:
* Do not generate unused type argument checks if the subtype class id
range to check is empty.
* Only generate a nullability check when checking that an instance
type argument is a subtype of non-nullable Object in null safe mode.
* Fix AbstractType::IsSubtypeOf so _Closure <: Function.
Fixes https://github.com/dart-lang/sdk/issues/46936
TEST=vm/cc/HierarchyInfo
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-release-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-release-x64-try,vm-kernel-nnbd-linux-release-x64-try,vm-kernel-tsan-linux-release-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try
Change-Id: Ic0310208d271ef04e778f070f420ae0abbdd47d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/210581
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
diff --git a/runtime/platform/assert.cc b/runtime/platform/assert.cc
index 5c7bf0b..c865467 100644
--- a/runtime/platform/assert.cc
+++ b/runtime/platform/assert.cc
@@ -18,7 +18,7 @@
void DynamicAssertionHelper::Print(const char* format,
va_list arguments,
- bool will_abort /* = false */) {
+ bool will_abort /* = false */) const {
// Take only the last 1KB of the file name if it is longer.
const intptr_t file_len = strlen(file_);
const intptr_t file_offset = (file_len > (1 * KB)) ? file_len - (1 * KB) : 0;
@@ -43,7 +43,7 @@
#endif // defined(DART_HOST_OS_ANDROID)
}
-void Assert::Fail(const char* format, ...) {
+void Assert::Fail(const char* format, ...) const {
va_list arguments;
va_start(arguments, format);
Print(format, arguments, /*will_abort=*/true);
@@ -55,7 +55,7 @@
abort();
}
-void Expect::Fail(const char* format, ...) {
+void Expect::Fail(const char* format, ...) const {
va_list arguments;
va_start(arguments, format);
Print(format, arguments);
diff --git a/runtime/platform/assert.h b/runtime/platform/assert.h
index bdf73c5..ce4d0cc 100644
--- a/runtime/platform/assert.h
+++ b/runtime/platform/assert.h
@@ -30,7 +30,9 @@
: file_(file), line_(line) {}
protected:
- void Print(const char* format, va_list arguments, bool will_abort = false);
+ void Print(const char* format,
+ va_list arguments,
+ bool will_abort = false) const;
const char* const file_;
const int line_;
@@ -42,7 +44,7 @@
public:
Assert(const char* file, int line) : DynamicAssertionHelper(file, line) {}
- DART_NORETURN void Fail(const char* format, ...) PRINTF_ATTRIBUTE(2, 3);
+ DART_NORETURN void Fail(const char* format, ...) const PRINTF_ATTRIBUTE(2, 3);
template <typename T>
T NotNull(const T p);
@@ -52,7 +54,7 @@
public:
Expect(const char* file, int line) : DynamicAssertionHelper(file, line) {}
- void Fail(const char* format, ...) PRINTF_ATTRIBUTE(2, 3);
+ void Fail(const char* format, ...) const PRINTF_ATTRIBUTE(2, 3);
#if defined(TESTING)
template <typename E, typename A>
diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
index 0224242..333b6e6 100644
--- a/runtime/vm/compiler/backend/il.cc
+++ b/runtime/vm/compiler/backend/il.cc
@@ -118,90 +118,108 @@
}
CidRangeVector& ranges = (*cid_ranges)[klass.id()];
if (ranges.length() == 0) {
- if (!FLAG_precompiled_mode) {
- BuildRangesForJIT(table, &ranges, klass, include_abstract, exclude_null);
- } else {
- BuildRangesFor(table, &ranges, klass, include_abstract, exclude_null);
- }
+ BuildRangesFor(table, &ranges, klass, include_abstract, exclude_null);
}
return ranges;
}
+class CidCheckerForRanges : public ValueObject {
+ public:
+ CidCheckerForRanges(Thread* thread,
+ ClassTable* table,
+ const Class& cls,
+ bool include_abstract,
+ bool exclude_null)
+ : thread_(thread),
+ table_(table),
+ supertype_(AbstractType::Handle(zone(), cls.RareType())),
+ include_abstract_(include_abstract),
+ exclude_null_(exclude_null),
+ to_check_(Class::Handle(zone())),
+ subtype_(AbstractType::Handle(zone())) {}
+
+ bool MayInclude(intptr_t cid) {
+ if (!table_->HasValidClassAt(cid)) return true;
+ if (cid == kTypeArgumentsCid) return true;
+ if (cid == kVoidCid) return true;
+ if (cid == kDynamicCid) return true;
+ if (cid == kNeverCid) return true;
+ if (!exclude_null_ && cid == kNullCid) return true;
+ to_check_ = table_->At(cid);
+ ASSERT(!to_check_.IsNull());
+ if (!include_abstract_ && to_check_.is_abstract()) return true;
+ return to_check_.IsTopLevel();
+ }
+
+ bool MustInclude(intptr_t cid) {
+ ASSERT(!MayInclude(cid));
+ if (cid == kNullCid) return false;
+ to_check_ = table_->At(cid);
+ subtype_ = to_check_.RareType();
+ // Create local zone because deep hierarchies may allocate lots of handles.
+ StackZone stack_zone(thread_);
+ HANDLESCOPE(thread_);
+ return subtype_.IsSubtypeOf(supertype_, Heap::kNew);
+ }
+
+ private:
+ Zone* zone() const { return thread_->zone(); }
+
+ Thread* const thread_;
+ ClassTable* const table_;
+ const AbstractType& supertype_;
+ const bool include_abstract_;
+ const bool exclude_null_;
+ Class& to_check_;
+ AbstractType& subtype_;
+};
+
// Build the ranges either for:
// "<obj> as <Type>", or
// "<obj> is <Type>"
-void HierarchyInfo::BuildRangesFor(ClassTable* table,
- CidRangeVector* ranges,
- const Class& klass,
- bool include_abstract,
- bool exclude_null) {
- Zone* zone = thread()->zone();
- const Type& dst_type = Type::Handle(zone, Type::RawCast(klass.RareType()));
- AbstractType& cls_type = AbstractType::Handle(zone);
- Class& cls = Class::Handle(zone);
- const intptr_t cid_count = table->NumCids();
-
+void HierarchyInfo::BuildRangesUsingClassTableFor(ClassTable* table,
+ CidRangeVector* ranges,
+ const Class& klass,
+ bool include_abstract,
+ bool exclude_null) {
+ CidCheckerForRanges checker(thread(), table, klass, include_abstract,
+ exclude_null);
// Iterate over all cids to find the ones to be included in the ranges.
+ const intptr_t cid_count = table->NumCids();
intptr_t start = -1;
intptr_t end = -1;
for (intptr_t cid = kInstanceCid; cid < cid_count; ++cid) {
- // Create local zone because deep hierarchies may allocate lots of handles
- // within one iteration of this loop.
- StackZone stack_zone(thread());
- HANDLESCOPE(thread());
-
// Some cases are "don't care", i.e., they may or may not be included,
// whatever yields the least number of ranges for efficiency.
- if (!table->HasValidClassAt(cid)) continue;
- if (cid == kTypeArgumentsCid) continue;
- if (cid == kVoidCid) continue;
- if (cid == kDynamicCid) continue;
- if (cid == kNeverCid) continue;
- if (cid == kNullCid && !exclude_null) continue;
- cls = table->At(cid);
- if (!include_abstract && cls.is_abstract()) continue;
- if (cls.IsTopLevel()) continue;
-
- // We are interested in [CidRange]es of subtypes.
- bool test_succeeded = false;
- if (cid == kNullCid) {
- ASSERT(exclude_null);
- test_succeeded = false;
- } else {
- cls_type = cls.RareType();
- test_succeeded = cls_type.IsSubtypeOf(dst_type, Heap::kNew);
- }
-
- if (test_succeeded) {
+ if (checker.MayInclude(cid)) continue;
+ if (checker.MustInclude(cid)) {
// On success, open a new or continue any open range.
if (start == -1) start = cid;
end = cid;
} else if (start != -1) {
// On failure, close any open range from start to end
// (the latter is the most recent succesful "do-care" cid).
- ASSERT(start <= end);
- CidRange range(start, end);
- ranges->Add(range);
- start = -1;
- end = -1;
+ ranges->Add({start, end});
+ start = end = -1;
}
}
// Construct last range if there is a open one.
if (start != -1) {
- ASSERT(start <= end);
- CidRange range(start, end);
- ranges->Add(range);
+ ranges->Add({start, end});
}
}
-void HierarchyInfo::BuildRangesForJIT(ClassTable* table,
- CidRangeVector* ranges,
- const Class& dst_klass,
- bool include_abstract,
- bool exclude_null) {
- if (dst_klass.InVMIsolateHeap()) {
- BuildRangesFor(table, ranges, dst_klass, include_abstract, exclude_null);
+void HierarchyInfo::BuildRangesFor(ClassTable* table,
+ CidRangeVector* ranges,
+ const Class& dst_klass,
+ bool include_abstract,
+ bool exclude_null) {
+ // Use the class table in cases where the direct subclasses and implementors
+ // are not filled out.
+ if (dst_klass.InVMIsolateHeap() || dst_klass.id() == kInstanceCid) {
+ BuildRangesUsingClassTableFor(table, ranges, dst_klass, include_abstract,
+ exclude_null);
return;
}
@@ -228,61 +246,50 @@
});
// Build ranges of all the cids.
- Class& klass = Class::Handle();
+ CidCheckerForRanges checker(thread(), table, dst_klass, include_abstract,
+ exclude_null);
intptr_t left_cid = -1;
- intptr_t last_cid = -1;
+ intptr_t right_cid = -1;
+ intptr_t previous_cid = -1;
for (intptr_t i = 0; i < cids.length(); ++i) {
- if (left_cid == -1) {
- left_cid = last_cid = cids[i];
- } else {
- const intptr_t current_cid = cids[i];
+ const intptr_t current_cid = cids[i];
+ if (current_cid == previous_cid) continue; // Skip duplicates.
- // Skip duplicates.
- if (current_cid == last_cid) continue;
+ // We sorted, after all!
+ RELEASE_ASSERT(previous_cid < current_cid);
- // Consecutive numbers cids are ok.
- if (current_cid == (last_cid + 1)) {
- last_cid = current_cid;
- } else {
- // We sorted, after all!
- RELEASE_ASSERT(last_cid < current_cid);
-
- intptr_t j = last_cid + 1;
- for (; j < current_cid; ++j) {
- if (table->HasValidClassAt(j)) {
- klass = table->At(j);
- if (!klass.IsTopLevel()) {
- // If we care about abstract classes also, we cannot skip over any
- // arbitrary abstract class, only those which are subtypes.
- if (include_abstract) {
- break;
- }
-
- // If the class is concrete we cannot skip over it.
- if (!klass.is_abstract()) {
- break;
- }
- }
- }
- }
-
- if (current_cid == j) {
- // If there's only abstract cids between [last_cid] and the
- // [current_cid] then we connect them.
- last_cid = current_cid;
- } else {
- // Finish the current open cid range and start a new one.
- ranges->Add(CidRange{left_cid, last_cid});
- left_cid = last_cid = current_cid;
+ if (left_cid != -1) {
+ ASSERT(previous_cid != -1);
+ // Check the cids between the previous cid from cids and this one.
+ for (intptr_t j = previous_cid + 1; j < current_cid; ++j) {
+ // Stop if we find a do-care class before reaching the current cid.
+ if (!checker.MayInclude(j)) {
+ ranges->Add({left_cid, right_cid});
+ left_cid = right_cid = -1;
+ break;
}
}
}
+ previous_cid = current_cid;
+
+ if (checker.MayInclude(current_cid)) continue;
+ if (checker.MustInclude(current_cid)) {
+ if (left_cid == -1) {
+ // Open a new range starting at this cid.
+ left_cid = current_cid;
+ }
+ right_cid = current_cid;
+ } else if (left_cid != -1) {
+ // Close the existing range.
+ ranges->Add({left_cid, right_cid});
+ left_cid = right_cid = -1;
+ }
}
// If there is an open cid-range which we haven't finished yet, we'll
// complete it.
if (left_cid != -1) {
- ranges->Add(CidRange{left_cid, last_cid});
+ ranges->Add(CidRange{left_cid, right_cid});
}
}
diff --git a/runtime/vm/compiler/backend/il.h b/runtime/vm/compiler/backend/il.h
index e4d883f..911426b 100644
--- a/runtime/vm/compiler/backend/il.h
+++ b/runtime/vm/compiler/backend/il.h
@@ -286,7 +286,22 @@
private:
// Does not use any hierarchy information available in the system but computes
- // it via O(n) class table traversal. The boolean parameters denote:
+ // it via O(n) class table traversal.
+ //
+ // The boolean parameters denote:
+ // include_abstract : if set, include abstract types (don't care otherwise)
+ // exclude_null : if set, exclude null types (don't care otherwise)
+ void BuildRangesUsingClassTableFor(ClassTable* table,
+ CidRangeVector* ranges,
+ const Class& klass,
+ bool include_abstract,
+ bool exclude_null);
+
+ // Uses hierarchy information stored in the [Class]'s direct_subclasses() and
+ // direct_implementors() arrays, unless that information is not available
+ // in which case we fall back to the class table.
+ //
+ // The boolean parameters denote:
// include_abstract : if set, include abstract types (don't care otherwise)
// exclude_null : if set, exclude null types (don't care otherwise)
void BuildRangesFor(ClassTable* table,
@@ -295,14 +310,6 @@
bool include_abstract,
bool exclude_null);
- // In JIT mode we use hierarchy information stored in the [RawClass]s
- // direct_subclasses_/direct_implementors_ arrays.
- void BuildRangesForJIT(ClassTable* table,
- CidRangeVector* ranges,
- const Class& klass,
- bool include_abstract,
- bool exclude_null);
-
std::unique_ptr<CidRangeVector[]> cid_subtype_ranges_nullable_;
std::unique_ptr<CidRangeVector[]> cid_subtype_ranges_abstract_nullable_;
std::unique_ptr<CidRangeVector[]> cid_subtype_ranges_nonnullable_;
diff --git a/runtime/vm/compiler/backend/il_test.cc b/runtime/vm/compiler/backend/il_test.cc
index 1e2e053..260d17d 100644
--- a/runtime/vm/compiler/backend/il_test.cc
+++ b/runtime/vm/compiler/backend/il_test.cc
@@ -6,7 +6,9 @@
#include <vector>
+#include "platform/text_buffer.h"
#include "platform/utils.h"
+#include "vm/class_id.h"
#include "vm/compiler/backend/block_builder.h"
#include "vm/compiler/backend/il_printer.h"
#include "vm/compiler/backend/il_test_helper.h"
@@ -375,4 +377,231 @@
EXPECT(!unbox->ComputeCanDeoptimize()); // Previously this reverted to true.
}
+static void WriteCidTo(intptr_t cid, BaseTextBuffer* buffer) {
+ ClassTable* const class_table = IsolateGroup::Current()->class_table();
+ buffer->Printf("%" Pd "", cid);
+ if (class_table->HasValidClassAt(cid)) {
+ const auto& cls = Class::Handle(class_table->At(cid));
+ buffer->Printf(" (%s", cls.ScrubbedNameCString());
+ if (cls.is_abstract()) {
+ buffer->AddString(", abstract");
+ }
+ buffer->AddString(")");
+ }
+}
+
+static void WriteCidRangeVectorTo(const CidRangeVector& ranges,
+ BaseTextBuffer* buffer) {
+ if (ranges.is_empty()) {
+ buffer->AddString("empty CidRangeVector");
+ return;
+ }
+ buffer->AddString("non-empty CidRangeVector:\n");
+ for (const auto& range : ranges) {
+ for (intptr_t cid = range.cid_start; cid <= range.cid_end; cid++) {
+ buffer->AddString(" * ");
+ WriteCidTo(cid, buffer);
+ buffer->AddString("\n");
+ }
+ }
+}
+
+static bool ExpectRangesContainCid(const Expect& expect,
+ const CidRangeVector& ranges,
+ intptr_t expected) {
+ for (const auto& range : ranges) {
+ for (intptr_t cid = range.cid_start; cid <= range.cid_end; cid++) {
+ if (expected == cid) return true;
+ }
+ }
+ TextBuffer buffer(128);
+ buffer.AddString("Expected CidRangeVector to include cid ");
+ WriteCidTo(expected, &buffer);
+ expect.Fail("%s", buffer.buffer());
+ return false;
+}
+
+static void RangesContainExpectedCids(const Expect& expect,
+ const CidRangeVector& ranges,
+ const GrowableArray<intptr_t>& expected) {
+ ASSERT(!ranges.is_empty());
+ ASSERT(!expected.is_empty());
+ {
+ TextBuffer buffer(128);
+ buffer.AddString("Checking that ");
+ WriteCidRangeVectorTo(ranges, &buffer);
+ buffer.AddString("includes cids:\n");
+ for (const intptr_t cid : expected) {
+ buffer.AddString(" * ");
+ WriteCidTo(cid, &buffer);
+ buffer.AddString("\n");
+ }
+ THR_Print("%s", buffer.buffer());
+ }
+ bool all_found = true;
+ for (const intptr_t cid : expected) {
+ if (!ExpectRangesContainCid(expect, ranges, cid)) {
+ all_found = false;
+ }
+ }
+ if (all_found) {
+ THR_Print("All expected cids included.\n\n");
+ }
+}
+
+#define RANGES_CONTAIN_EXPECTED_CIDS(ranges, cids) \
+ RangesContainExpectedCids(dart::Expect(__FILE__, __LINE__), ranges, cids)
+
+ISOLATE_UNIT_TEST_CASE(HierarchyInfo_Object_Subtype) {
+ HierarchyInfo hi(thread);
+ const auto& type =
+ Type::Handle(IsolateGroup::Current()->object_store()->object_type());
+ const bool is_nullable = Instance::NullIsAssignableTo(type);
+ EXPECT(hi.CanUseSubtypeRangeCheckFor(type));
+ const auto& cls = Class::Handle(type.type_class());
+
+ ClassTable* const class_table = thread->isolate_group()->class_table();
+ const intptr_t num_cids = class_table->NumCids();
+ auto& to_check = Class::Handle(thread->zone());
+ auto& rare_type = AbstractType::Handle(thread->zone());
+
+ GrowableArray<intptr_t> expected_concrete_cids;
+ GrowableArray<intptr_t> expected_abstract_cids;
+ for (intptr_t cid = kInstanceCid; cid < num_cids; cid++) {
+ if (!class_table->HasValidClassAt(cid)) continue;
+ if (cid == kNullCid && is_nullable) continue;
+ to_check = class_table->At(cid);
+ // Only add concrete classes.
+ if (to_check.is_abstract()) {
+ expected_abstract_cids.Add(cid);
+ } else {
+ expected_concrete_cids.Add(cid);
+ }
+ if (cid != kTypeArgumentsCid) { // Cannot call RareType() on this.
+ rare_type = to_check.RareType();
+ EXPECT(rare_type.IsSubtypeOf(type, Heap::kNew));
+ }
+ }
+
+ const CidRangeVector& concrete_range = hi.SubtypeRangesForClass(
+ cls, /*include_abstract=*/false, /*exclude_null=*/!is_nullable);
+ RANGES_CONTAIN_EXPECTED_CIDS(concrete_range, expected_concrete_cids);
+
+ GrowableArray<intptr_t> expected_cids;
+ expected_cids.AddArray(expected_concrete_cids);
+ expected_cids.AddArray(expected_abstract_cids);
+ const CidRangeVector& abstract_range = hi.SubtypeRangesForClass(
+ cls, /*include_abstract=*/true, /*exclude_null=*/!is_nullable);
+ RANGES_CONTAIN_EXPECTED_CIDS(abstract_range, expected_cids);
+}
+
+ISOLATE_UNIT_TEST_CASE(HierarchyInfo_Function_Subtype) {
+ HierarchyInfo hi(thread);
+ const auto& type =
+ Type::Handle(IsolateGroup::Current()->object_store()->function_type());
+ EXPECT(hi.CanUseSubtypeRangeCheckFor(type));
+ const auto& cls = Class::Handle(type.type_class());
+
+ GrowableArray<intptr_t> expected_concrete_cids;
+ expected_concrete_cids.Add(kClosureCid);
+
+ GrowableArray<intptr_t> expected_abstract_cids;
+ expected_abstract_cids.Add(type.type_class_id());
+
+ const CidRangeVector& concrete_range = hi.SubtypeRangesForClass(
+ cls, /*include_abstract=*/false, /*exclude_null=*/true);
+ RANGES_CONTAIN_EXPECTED_CIDS(concrete_range, expected_concrete_cids);
+
+ GrowableArray<intptr_t> expected_cids;
+ expected_cids.AddArray(expected_concrete_cids);
+ expected_cids.AddArray(expected_abstract_cids);
+ const CidRangeVector& abstract_range = hi.SubtypeRangesForClass(
+ cls, /*include_abstract=*/true, /*exclude_null=*/true);
+ RANGES_CONTAIN_EXPECTED_CIDS(abstract_range, expected_cids);
+}
+
+ISOLATE_UNIT_TEST_CASE(HierarchyInfo_Num_Subtype) {
+ HierarchyInfo hi(thread);
+ const auto& num_type = Type::Handle(Type::Number());
+ const auto& int_type = Type::Handle(Type::IntType());
+ const auto& double_type = Type::Handle(Type::Double());
+ EXPECT(hi.CanUseSubtypeRangeCheckFor(num_type));
+ const auto& cls = Class::Handle(num_type.type_class());
+
+ GrowableArray<intptr_t> expected_concrete_cids;
+ expected_concrete_cids.Add(kSmiCid);
+ expected_concrete_cids.Add(kMintCid);
+ expected_concrete_cids.Add(kDoubleCid);
+
+ GrowableArray<intptr_t> expected_abstract_cids;
+ expected_abstract_cids.Add(num_type.type_class_id());
+ expected_abstract_cids.Add(int_type.type_class_id());
+ expected_abstract_cids.Add(double_type.type_class_id());
+
+ const CidRangeVector& concrete_range = hi.SubtypeRangesForClass(
+ cls, /*include_abstract=*/false, /*exclude_null=*/true);
+ RANGES_CONTAIN_EXPECTED_CIDS(concrete_range, expected_concrete_cids);
+
+ GrowableArray<intptr_t> expected_cids;
+ expected_cids.AddArray(expected_concrete_cids);
+ expected_cids.AddArray(expected_abstract_cids);
+ const CidRangeVector& abstract_range = hi.SubtypeRangesForClass(
+ cls, /*include_abstract=*/true, /*exclude_null=*/true);
+ RANGES_CONTAIN_EXPECTED_CIDS(abstract_range, expected_cids);
+}
+
+ISOLATE_UNIT_TEST_CASE(HierarchyInfo_Int_Subtype) {
+ HierarchyInfo hi(thread);
+ const auto& type = Type::Handle(Type::IntType());
+ EXPECT(hi.CanUseSubtypeRangeCheckFor(type));
+ const auto& cls = Class::Handle(type.type_class());
+
+ GrowableArray<intptr_t> expected_concrete_cids;
+ expected_concrete_cids.Add(kSmiCid);
+ expected_concrete_cids.Add(kMintCid);
+
+ GrowableArray<intptr_t> expected_abstract_cids;
+ expected_abstract_cids.Add(type.type_class_id());
+
+ const CidRangeVector& concrete_range = hi.SubtypeRangesForClass(
+ cls, /*include_abstract=*/false, /*exclude_null=*/true);
+ RANGES_CONTAIN_EXPECTED_CIDS(concrete_range, expected_concrete_cids);
+
+ GrowableArray<intptr_t> expected_cids;
+ expected_cids.AddArray(expected_concrete_cids);
+ expected_cids.AddArray(expected_abstract_cids);
+ const CidRangeVector& abstract_range = hi.SubtypeRangesForClass(
+ cls, /*include_abstract=*/true, /*exclude_null=*/true);
+ RANGES_CONTAIN_EXPECTED_CIDS(abstract_range, expected_cids);
+}
+
+ISOLATE_UNIT_TEST_CASE(HierarchyInfo_String_Subtype) {
+ HierarchyInfo hi(thread);
+ const auto& type = Type::Handle(Type::StringType());
+ EXPECT(hi.CanUseSubtypeRangeCheckFor(type));
+ const auto& cls = Class::Handle(type.type_class());
+
+ GrowableArray<intptr_t> expected_concrete_cids;
+ expected_concrete_cids.Add(kOneByteStringCid);
+ expected_concrete_cids.Add(kTwoByteStringCid);
+ expected_concrete_cids.Add(kExternalOneByteStringCid);
+ expected_concrete_cids.Add(kExternalTwoByteStringCid);
+
+ GrowableArray<intptr_t> expected_abstract_cids;
+ expected_abstract_cids.Add(type.type_class_id());
+
+ const CidRangeVector& concrete_range = hi.SubtypeRangesForClass(
+ cls, /*include_abstract=*/false, /*exclude_null=*/true);
+ THR_Print("Checking concrete subtype ranges for String\n");
+ RANGES_CONTAIN_EXPECTED_CIDS(concrete_range, expected_concrete_cids);
+
+ GrowableArray<intptr_t> expected_cids;
+ expected_cids.AddArray(expected_concrete_cids);
+ expected_cids.AddArray(expected_abstract_cids);
+ const CidRangeVector& abstract_range = hi.SubtypeRangesForClass(
+ cls, /*include_abstract=*/true, /*exclude_null=*/true);
+ THR_Print("Checking concrete and abstract subtype ranges for String\n");
+ RANGES_CONTAIN_EXPECTED_CIDS(abstract_range, expected_cids);
+}
+
} // namespace dart
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 9b7ff28..d45a059 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -20453,35 +20453,35 @@
return false;
}
// Function types cannot be handled by Class::IsSubtypeOf().
- const bool other_is_dart_function_type = other.IsDartFunctionType();
- if (other_is_dart_function_type || other.IsFunctionType()) {
- if (IsFunctionType()) {
+ if (other.IsDartFunctionType()) {
+ // Any type that can be the type of a closure is a subtype of Function.
+ if (IsDartFunctionType() || IsDartClosureType() || IsFunctionType()) {
+ if (isolate_group->use_strict_null_safety_checks() && IsNullable()) {
+ return !other.IsNonNullable();
+ }
+ return true;
+ }
+ // Fall through.
+ }
+ if (IsFunctionType()) {
+ if (other.IsFunctionType()) {
+ // Check for two function types.
if (isolate_group->use_strict_null_safety_checks() && IsNullable() &&
other.IsNonNullable()) {
return false;
}
- if (other_is_dart_function_type) {
- return true;
- }
- // Check for two function types.
return FunctionType::Cast(*this).IsSubtypeOf(FunctionType::Cast(other),
space);
}
- if (other.IsFunctionType()) {
- // [this] is not a function type. Therefore, non-function type [this]
- // cannot be a subtype of function type [other].
- // This check is needed to avoid falling through to class-based type
- // tests, which yield incorrect result if [this] = _Closure class,
- // and [other] is a function type, because class of a function type is
- // also _Closure.
- return false;
- }
- }
- if (IsFunctionType()) {
// Apply additional subtyping rules if 'other' is 'FutureOr'.
if (IsSubtypeOfFutureOr(zone, other, space, trail)) {
return true;
}
+ // All possible supertypes for FunctionType have been checked.
+ return false;
+ } else if (other.IsFunctionType()) {
+ // FunctionTypes can only be subtyped by other FunctionTypes, so don't
+ // fall through to class-based type tests.
return false;
}
const Class& type_cls = Class::Handle(zone, type_class());
diff --git a/runtime/vm/object_test.cc b/runtime/vm/object_test.cc
index a2e7385..fa73cff 100644
--- a/runtime/vm/object_test.cc
+++ b/runtime/vm/object_test.cc
@@ -5304,6 +5304,16 @@
EXPECT(!m.IsSubtypeOf(t, Heap::kNew));
}
+ISOLATE_UNIT_TEST_CASE(ClosureType_SubtypeOfFunctionType) {
+ const auto& closure_class =
+ Class::Handle(IsolateGroup::Current()->object_store()->closure_class());
+ const auto& closure_type = Type::Handle(closure_class.DeclarationType());
+ const auto& function_type =
+ Type::Handle(IsolateGroup::Current()->object_store()->function_type());
+
+ EXPECT(closure_type.IsSubtypeOf(function_type, Heap::kNew));
+}
+
TEST_CASE(Class_GetInstantiationOf) {
const char* kScript = R"(
class B<T> {}
diff --git a/runtime/vm/type_testing_stubs.cc b/runtime/vm/type_testing_stubs.cc
index 47598e4..a7de67d 100644
--- a/runtime/vm/type_testing_stubs.cc
+++ b/runtime/vm/type_testing_stubs.cc
@@ -849,6 +849,11 @@
const CidRangeVector& ranges =
hi->SubtypeRangesForClass(type_class, /*include_abstract=*/false,
!Instance::NullIsAssignableTo(type));
+ if (ranges.is_empty()) {
+ // Fall through and signal type argument checks should not be generated.
+ CommentCheckedClasses(assembler, ranges);
+ return false;
+ }
if (!type_class.is_implemented()) {
ASSERT(type_class.is_finalized());
const intptr_t tav_offset =
@@ -966,52 +971,65 @@
__ BranchIf(NOT_EQUAL, check_failed);
__ Bind(&is_dynamic);
} else {
- const Class& type_class = Class::Handle(type_arg.type_class());
- const bool null_is_assignable = Instance::NullIsAssignableTo(type_arg);
- const CidRangeVector& ranges =
- hi->SubtypeRangesForClass(type_class,
- /*include_abstract=*/true,
- /*exclude_null=*/!null_is_assignable);
-
__ LoadCompressedFieldFromOffset(
TTSInternalRegs::kScratchReg,
TTSInternalRegs::kInstanceTypeArgumentsReg,
compiler::target::TypeArguments::type_at_offset(
type_param_value_offset_i));
- __ LoadCompressedFieldFromOffset(
- TTSInternalRegs::kScratchReg, TTSInternalRegs::kScratchReg,
- compiler::target::Type::type_class_id_offset());
-
- compiler::Label is_subtype;
- __ SmiUntag(TTSInternalRegs::kScratchReg);
- if (null_is_assignable) {
- __ CompareImmediate(TTSInternalRegs::kScratchReg, kNullCid);
- __ BranchIf(EQUAL, &is_subtype);
- }
- // Never is a bottom type.
- __ CompareImmediate(TTSInternalRegs::kScratchReg, kNeverCid);
- __ BranchIf(EQUAL, &is_subtype);
- BuildOptimizedSubtypeRangeCheck(assembler, ranges,
- TTSInternalRegs::kScratchReg, &is_subtype,
- check_failed);
- __ Bind(&is_subtype);
-
- // Weak NNBD mode uses LEGACY_SUBTYPE which ignores nullability.
- // We don't need to check nullability of LHS for nullable and legacy RHS
- // ("Right Legacy", "Right Nullable" rules).
- if (IsolateGroup::Current()->use_strict_null_safety_checks() &&
- !type_arg.IsNullable() && !type_arg.IsLegacy()) {
- // Nullable type is not a subtype of non-nullable type.
- // TODO(dartbug.com/40736): Allocate a register for instance type argument
- // and avoid reloading it.
- __ LoadCompressedFieldFromOffset(
- TTSInternalRegs::kScratchReg,
- TTSInternalRegs::kInstanceTypeArgumentsReg,
- compiler::target::TypeArguments::type_at_offset(
- type_param_value_offset_i));
+ if (type_arg.IsObjectType()) {
+ // Just check the nullability, since this must be non-nullable Object
+ // and we must be in null safe mode.
+ ASSERT(IsolateGroup::Current()->use_strict_null_safety_checks() &&
+ type_arg.IsNonNullable());
__ CompareTypeNullabilityWith(TTSInternalRegs::kScratchReg,
compiler::target::Nullability::kNullable);
__ BranchIf(EQUAL, check_failed);
+ } else {
+ __ LoadCompressedFieldFromOffset(
+ TTSInternalRegs::kScratchReg, TTSInternalRegs::kScratchReg,
+ compiler::target::Type::type_class_id_offset());
+
+ const Class& type_class = Class::Handle(type_arg.type_class());
+ const bool null_is_assignable = Instance::NullIsAssignableTo(type_arg);
+ const CidRangeVector& ranges =
+ hi->SubtypeRangesForClass(type_class,
+ /*include_abstract=*/true,
+ /*exclude_null=*/!null_is_assignable);
+
+ compiler::Label is_subtype;
+ __ SmiUntag(TTSInternalRegs::kScratchReg);
+ // Never is a bottom type.
+ __ CompareImmediate(TTSInternalRegs::kScratchReg, kNeverCid);
+ __ BranchIf(EQUAL, &is_subtype);
+ if (null_is_assignable) {
+ __ CompareImmediate(TTSInternalRegs::kScratchReg, kNullCid);
+ __ BranchIf(EQUAL, &is_subtype);
+ }
+ compiler::Label check_nullability;
+ BuildOptimizedSubtypeRangeCheck(assembler, ranges,
+ TTSInternalRegs::kScratchReg,
+ &check_nullability, check_failed);
+ __ Bind(&check_nullability);
+
+ // Weak NNBD mode uses LEGACY_SUBTYPE which ignores nullability.
+ // We don't need to check nullability of LHS for nullable and legacy RHS
+ // ("Right Legacy", "Right Nullable" rules).
+ if (IsolateGroup::Current()->use_strict_null_safety_checks() &&
+ type_arg.IsNonNullable()) {
+ // Nullable type is not a subtype of non-nullable type.
+ // TODO(dartbug.com/40736): Allocate a register for instance type
+ // argument and avoid reloading it.
+ __ LoadCompressedFieldFromOffset(
+ TTSInternalRegs::kScratchReg,
+ TTSInternalRegs::kInstanceTypeArgumentsReg,
+ compiler::target::TypeArguments::type_at_offset(
+ type_param_value_offset_i));
+ __ CompareTypeNullabilityWith(TTSInternalRegs::kScratchReg,
+ compiler::target::Nullability::kNullable);
+ __ BranchIf(EQUAL, check_failed);
+ }
+
+ __ Bind(&is_subtype);
}
}
}
diff --git a/runtime/vm/type_testing_stubs.h b/runtime/vm/type_testing_stubs.h
index 74cc0c0..40534c7 100644
--- a/runtime/vm/type_testing_stubs.h
+++ b/runtime/vm/type_testing_stubs.h
@@ -83,10 +83,16 @@
const Type& type,
const Class& type_class);
- // Falls through or jumps to load_succeeded if load succeeds, otherwise jumps
- // to load_failed. Returns from the stub for checked cid ranges which do not
- // require checking the instance type arguments. Returns whether any cid
- // ranges require type argument checking.
+ // Returns whether any cid ranges require type argument checking.
+ //
+ // If any do, then returns from the stub if any checks that do not need
+ // type argument checking succeed, falls through or jumps to load_succeeded if
+ // loading the type arguments succeeds, and otherwise jumps to load_failed.
+ // That is, code that uses the type arguments should follow immediately.
+ //
+ // If none do, then falls through or jumps to load_failed if the checks fail,
+ // else returns from the stub if the checks are successful. That is, code
+ // that handles the failure case (like calling the slow stub) should follow.
static bool BuildLoadInstanceTypeArguments(
compiler::Assembler* assembler,
HierarchyInfo* hi,
