Skip to content

Commit 8d26e3d

Browse files
sjindel-googlecommit-bot@chromium.org
authored andcommitted
[vm] Don't insert CheckedSmi* instructions when the interface target suggests it's wrong.
Change-Id: I97e45dc5c74f0d6e9193c0fa95dd679a42133f78 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138521 Reviewed-by: Victor Agnez Lima <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Commit-Queue: Samir Jindel <[email protected]>
1 parent f6b0a70 commit 8d26e3d

File tree

4 files changed

+49
-18
lines changed

4 files changed

+49
-18
lines changed

runtime/vm/compiler/aot/aot_call_specializer.cc

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,13 @@ bool AotCallSpecializer::TryReplaceWithHaveSameRuntimeType(
220220

221221
static bool HasLikelySmiOperand(InstanceCallInstr* instr) {
222222
ASSERT(instr->type_args_len() == 0);
223+
224+
// If Smi is not assignable to the interface target of the call, the receiver
225+
// is definitely not a Smi.
226+
if (instr->HasNonSmiAssignableInterface(Thread::Current()->zone())) {
227+
return false;
228+
}
229+
223230
// Phis with at least one known smi are // guessed to be likely smi as well.
224231
for (intptr_t i = 0; i < instr->ArgumentCount(); ++i) {
225232
PhiInstr* phi = instr->ArgumentAt(i)->AsPhi();
@@ -472,9 +479,15 @@ bool AotCallSpecializer::TryOptimizeIntegerOperation(TemplateDartCall<0>* instr,
472479
CompileType* right_type = right_value->Type();
473480

474481
const bool is_equality_op = Token::IsEqualityOperator(op_kind);
475-
const bool has_nullable_int_args =
482+
bool has_nullable_int_args =
476483
left_type->IsNullableInt() && right_type->IsNullableInt();
477484

485+
if (auto* call = instr->AsInstanceCall()) {
486+
if (call->HasNonSmiAssignableInterface(zone())) {
487+
has_nullable_int_args = false;
488+
}
489+
}
490+
478491
// NOTE: We cannot use strict comparisons if the receiver has an overridden
479492
// == operator or if either side can be a double, since 1.0 == 1.
480493
const bool can_use_strict_compare =

runtime/vm/compiler/backend/il.cc

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4287,19 +4287,26 @@ static RawCode* TwoArgsSmiOpInlineCacheEntry(Token::Kind kind) {
42874287
}
42884288
}
42894289

4290+
bool InstanceCallBaseInstr::HasNonSmiAssignableInterface(Zone* zone) const {
4291+
if (!interface_target().IsNull()) {
4292+
const AbstractType& target_type = AbstractType::Handle(
4293+
zone, Class::Handle(zone, interface_target().Owner()).RareType());
4294+
if (!CompileType::Smi().IsAssignableTo(target_type)) {
4295+
return true;
4296+
}
4297+
}
4298+
return false;
4299+
}
4300+
42904301
void InstanceCallBaseInstr::UpdateReceiverSminess(Zone* zone) {
42914302
if (FLAG_precompiled_mode && !receiver_is_not_smi()) {
42924303
if (Receiver()->Type()->IsNotSmi()) {
42934304
set_receiver_is_not_smi(true);
42944305
return;
42954306
}
42964307

4297-
if (!interface_target().IsNull()) {
4298-
const AbstractType& target_type = AbstractType::Handle(
4299-
zone, Class::Handle(zone, interface_target().Owner()).RareType());
4300-
if (!CompileType::Smi().IsAssignableTo(target_type)) {
4301-
set_receiver_is_not_smi(true);
4302-
}
4308+
if (HasNonSmiAssignableInterface(zone)) {
4309+
set_receiver_is_not_smi(true);
43034310
}
43044311
}
43054312
}

runtime/vm/compiler/backend/il.h

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class UnboxIntegerInstr;
6161
namespace compiler {
6262
class BlockBuilder;
6363
struct TableSelector;
64-
}
64+
} // namespace compiler
6565

6666
class Value : public ZoneAllocated {
6767
public:
@@ -239,9 +239,7 @@ class HierarchyInfo : public ThreadStackResource {
239239
thread->set_hierarchy_info(this);
240240
}
241241

242-
~HierarchyInfo() {
243-
thread()->set_hierarchy_info(NULL);
244-
}
242+
~HierarchyInfo() { thread()->set_hierarchy_info(NULL); }
245243

246244
const CidRangeVector& SubtypeRangesForClass(const Class& klass,
247245
bool include_abstract,
@@ -3854,6 +3852,8 @@ class InstanceCallBaseInstr : public TemplateDartCall<0> {
38543852
// interface target, CompileType and hints from TFA.
38553853
void UpdateReceiverSminess(Zone* zone);
38563854

3855+
bool HasNonSmiAssignableInterface(Zone* zone) const;
3856+
38573857
protected:
38583858
friend class CallSpecializer;
38593859
void set_ic_data(ICData* value) { ic_data_ = value; }
@@ -7047,6 +7047,10 @@ class CheckedSmiOpInstr : public TemplateDefinition<2, Throws> {
70477047
TemplateDartCall<0>* call)
70487048
: TemplateDefinition(call->deopt_id()), call_(call), op_kind_(op_kind) {
70497049
ASSERT(call->type_args_len() == 0);
7050+
ASSERT(!call->IsInstanceCallBase() ||
7051+
!call->AsInstanceCallBase()->HasNonSmiAssignableInterface(
7052+
Thread::Current()->zone()));
7053+
70507054
SetInputAt(0, left);
70517055
SetInputAt(1, right);
70527056
}
@@ -7086,6 +7090,10 @@ class CheckedSmiComparisonInstr : public TemplateComparison<2, Throws> {
70867090
call_(call),
70877091
is_negated_(false) {
70887092
ASSERT(call->type_args_len() == 0);
7093+
ASSERT(!call->IsInstanceCallBase() ||
7094+
!call->AsInstanceCallBase()->HasNonSmiAssignableInterface(
7095+
Thread::Current()->zone()));
7096+
70897097
SetInputAt(0, left);
70907098
SetInputAt(1, right);
70917099
}

runtime/vm/compiler/call_specializer.cc

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -125,13 +125,16 @@ bool CallSpecializer::TryCreateICData(InstanceCallInstr* call) {
125125
Token::IsEqualityOperator(op_kind) ||
126126
Token::IsBinaryOperator(op_kind)) {
127127
// Guess cid: if one of the inputs is a number assume that the other
128-
// is a number of same type.
129-
const intptr_t cid_0 = class_ids[0];
130-
const intptr_t cid_1 = class_ids[1];
131-
if ((cid_0 == kDynamicCid) && (IsNumberCid(cid_1))) {
132-
class_ids[0] = cid_1;
133-
} else if (IsNumberCid(cid_0) && (cid_1 == kDynamicCid)) {
134-
class_ids[1] = cid_0;
128+
// is a number of same type, unless the interface target tells us this
129+
// is impossible.
130+
if (!call->HasNonSmiAssignableInterface(zone())) {
131+
const intptr_t cid_0 = class_ids[0];
132+
const intptr_t cid_1 = class_ids[1];
133+
if ((cid_0 == kDynamicCid) && (IsNumberCid(cid_1))) {
134+
class_ids[0] = cid_1;
135+
} else if (IsNumberCid(cid_0) && (cid_1 == kDynamicCid)) {
136+
class_ids[1] = cid_0;
137+
}
135138
}
136139
}
137140
}

0 commit comments

Comments
 (0)