Skip to content

Commit 7ab210e

Browse files
mkustermanncommit-bot@chromium.org
mkustermann
authored and
[email protected]
committed
[VM] Allow EqualityCompare/RelationalOp/BinaryInt64OpInstr to take constants as inputs
Issue #31798 Change-Id: I3a64fc9e6a038ae28e3a98149b0494ebe254fd73 Reviewed-on: https://dart-review.googlesource.com/33780 Commit-Queue: Martin Kustermann <[email protected]> Reviewed-by: Vyacheslav Egorov <[email protected]>
1 parent 2da64ef commit 7ab210e

File tree

3 files changed

+94
-63
lines changed

3 files changed

+94
-63
lines changed

‎runtime/vm/compiler/backend/il.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2722,6 +2722,12 @@ class ConstantInstr : public TemplateDefinition<0, NoThrow, Pure> {
27222722
representation() == kUnboxedInt64;
27232723
}
27242724

2725+
int64_t GetUnboxedSignedIntegerConstantValue() const {
2726+
ASSERT(IsUnboxedSignedIntegerConstant());
2727+
return value_.IsSmi() ? Smi::Cast(value_).Value()
2728+
: Mint::Cast(value_).value();
2729+
}
2730+
27252731
void EmitMoveToLocation(FlowGraphCompiler* compiler,
27262732
const Location& destination,
27272733
Register tmp = kNoRegister);

‎runtime/vm/compiler/backend/il_arm64.cc

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -529,20 +529,34 @@ static void EmitBranchOnCondition(FlowGraphCompiler* compiler,
529529
}
530530
}
531531

532-
static Condition EmitSmiComparisonOp(FlowGraphCompiler* compiler,
533-
LocationSummary* locs,
534-
Token::Kind kind) {
532+
static Condition EmitInt64ComparisonOp(FlowGraphCompiler* compiler,
533+
LocationSummary* locs,
534+
Token::Kind kind) {
535535
Location left = locs->in(0);
536536
Location right = locs->in(1);
537537
ASSERT(!left.IsConstant() || !right.IsConstant());
538538

539539
Condition true_condition = TokenKindToSmiCondition(kind);
540+
if (left.IsConstant() || right.IsConstant()) {
541+
// Ensure constant is on the right.
542+
ConstantInstr* right_constant = NULL;
543+
if (left.IsConstant()) {
544+
right_constant = left.constant_instruction();
545+
Location tmp = right;
546+
right = left;
547+
left = tmp;
548+
true_condition = FlipCondition(true_condition);
549+
} else {
550+
right_constant = right.constant_instruction();
551+
}
540552

541-
if (left.IsConstant()) {
542-
__ CompareObject(right.reg(), left.constant());
543-
true_condition = FlipCondition(true_condition);
544-
} else if (right.IsConstant()) {
545-
__ CompareObject(left.reg(), right.constant());
553+
if (right_constant->IsUnboxedSignedIntegerConstant()) {
554+
__ CompareImmediate(
555+
left.reg(), right_constant->GetUnboxedSignedIntegerConstantValue());
556+
} else {
557+
ASSERT(right_constant->representation() == kTagged);
558+
__ CompareObject(left.reg(), right.constant());
559+
}
546560
} else {
547561
__ CompareRegisters(left.reg(), right.reg());
548562
}
@@ -561,7 +575,7 @@ LocationSummary* EqualityCompareInstr::MakeLocationSummary(Zone* zone,
561575
locs->set_out(0, Location::RequiresRegister());
562576
return locs;
563577
}
564-
if (operation_cid() == kSmiCid) {
578+
if (operation_cid() == kSmiCid || operation_cid() == kMintCid) {
565579
const intptr_t kNumTemps = 0;
566580
LocationSummary* locs = new (zone)
567581
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
@@ -617,8 +631,8 @@ static Condition EmitDoubleComparisonOp(FlowGraphCompiler* compiler,
617631

618632
Condition EqualityCompareInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
619633
BranchLabels labels) {
620-
if (operation_cid() == kSmiCid) {
621-
return EmitSmiComparisonOp(compiler, locs(), kind());
634+
if (operation_cid() == kSmiCid || operation_cid() == kMintCid) {
635+
return EmitInt64ComparisonOp(compiler, locs(), kind());
622636
} else {
623637
ASSERT(operation_cid() == kDoubleCid);
624638
return EmitDoubleComparisonOp(compiler, locs(), labels, kind());
@@ -718,23 +732,27 @@ LocationSummary* RelationalOpInstr::MakeLocationSummary(Zone* zone,
718732
summary->set_out(0, Location::RequiresRegister());
719733
return summary;
720734
}
721-
ASSERT(operation_cid() == kSmiCid);
722-
LocationSummary* summary = new (zone)
723-
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
724-
summary->set_in(0, Location::RegisterOrConstant(left()));
725-
// Only one input can be a constant operand. The case of two constant
726-
// operands should be handled by constant propagation.
727-
summary->set_in(1, summary->in(0).IsConstant()
728-
? Location::RequiresRegister()
729-
: Location::RegisterOrConstant(right()));
730-
summary->set_out(0, Location::RequiresRegister());
731-
return summary;
735+
if (operation_cid() == kSmiCid || operation_cid() == kMintCid) {
736+
LocationSummary* summary = new (zone)
737+
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
738+
summary->set_in(0, Location::RegisterOrConstant(left()));
739+
// Only one input can be a constant operand. The case of two constant
740+
// operands should be handled by constant propagation.
741+
summary->set_in(1, summary->in(0).IsConstant()
742+
? Location::RequiresRegister()
743+
: Location::RegisterOrConstant(right()));
744+
summary->set_out(0, Location::RequiresRegister());
745+
return summary;
746+
}
747+
748+
UNREACHABLE();
749+
return NULL;
732750
}
733751

734752
Condition RelationalOpInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
735753
BranchLabels labels) {
736-
if (operation_cid() == kSmiCid) {
737-
return EmitSmiComparisonOp(compiler, locs(), kind());
754+
if (operation_cid() == kSmiCid || operation_cid() == kMintCid) {
755+
return EmitInt64ComparisonOp(compiler, locs(), kind());
738756
} else {
739757
ASSERT(operation_cid() == kDoubleCid);
740758
return EmitDoubleComparisonOp(compiler, locs(), labels, kind());
@@ -2979,7 +2997,7 @@ LocationSummary* CheckedSmiComparisonInstr::MakeLocationSummary(
29792997
Condition CheckedSmiComparisonInstr::EmitComparisonCode(
29802998
FlowGraphCompiler* compiler,
29812999
BranchLabels labels) {
2982-
return EmitSmiComparisonOp(compiler, locs(), kind());
3000+
return EmitInt64ComparisonOp(compiler, locs(), kind());
29833001
}
29843002

29853003
#define EMIT_SMI_CHECK \

‎runtime/vm/compiler/backend/il_x64.cc

Lines changed: 45 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -454,15 +454,6 @@ static Condition TokenKindToIntCondition(Token::Kind kind) {
454454
LocationSummary* EqualityCompareInstr::MakeLocationSummary(Zone* zone,
455455
bool opt) const {
456456
const intptr_t kNumInputs = 2;
457-
if (operation_cid() == kMintCid) {
458-
const intptr_t kNumTemps = 0;
459-
LocationSummary* locs = new (zone)
460-
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
461-
locs->set_in(0, Location::RequiresRegister());
462-
locs->set_in(1, Location::RequiresRegister());
463-
locs->set_out(0, Location::RequiresRegister());
464-
return locs;
465-
}
466457
if (operation_cid() == kDoubleCid) {
467458
const intptr_t kNumTemps = 0;
468459
LocationSummary* locs = new (zone)
@@ -472,7 +463,7 @@ LocationSummary* EqualityCompareInstr::MakeLocationSummary(Zone* zone,
472463
locs->set_out(0, Location::RequiresRegister());
473464
return locs;
474465
}
475-
if (operation_cid() == kSmiCid) {
466+
if (operation_cid() == kSmiCid || operation_cid() == kMintCid) {
476467
const intptr_t kNumTemps = 0;
477468
LocationSummary* locs = new (zone)
478469
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
@@ -562,12 +553,26 @@ static Condition EmitInt64ComparisonOp(FlowGraphCompiler* compiler,
562553
ASSERT(!left.IsConstant() || !right.IsConstant());
563554

564555
Condition true_condition = TokenKindToIntCondition(kind);
556+
if (left.IsConstant() || right.IsConstant()) {
557+
// Ensure constant is on the right.
558+
ConstantInstr* constant = NULL;
559+
if (left.IsConstant()) {
560+
constant = left.constant_instruction();
561+
Location tmp = right;
562+
right = left;
563+
left = tmp;
564+
true_condition = FlipCondition(true_condition);
565+
} else {
566+
constant = right.constant_instruction();
567+
}
565568

566-
if (left.IsConstant()) {
567-
__ CompareObject(right.reg(), left.constant());
568-
true_condition = FlipCondition(true_condition);
569-
} else if (right.IsConstant()) {
570-
__ CompareObject(left.reg(), right.constant());
569+
if (constant->IsUnboxedSignedIntegerConstant()) {
570+
__ cmpq(left.reg(),
571+
Immediate(constant->GetUnboxedSignedIntegerConstantValue()));
572+
} else {
573+
ASSERT(constant->representation() == kTagged);
574+
__ CompareObject(left.reg(), right.constant());
575+
}
571576
} else if (right.IsStackSlot()) {
572577
__ cmpq(left.reg(), right.ToStackSlotAddress());
573578
} else {
@@ -741,30 +746,26 @@ LocationSummary* RelationalOpInstr::MakeLocationSummary(Zone* zone,
741746
summary->set_in(1, Location::RequiresFpuRegister());
742747
summary->set_out(0, Location::RequiresRegister());
743748
return summary;
744-
} else if (operation_cid() == kMintCid) {
749+
}
750+
if (operation_cid() == kSmiCid || operation_cid() == kMintCid) {
745751
LocationSummary* summary = new (zone)
746752
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
747-
summary->set_in(0, Location::RequiresRegister());
748-
summary->set_in(1, Location::RequiresRegister());
753+
summary->set_in(0, Location::RegisterOrConstant(left()));
754+
// Only one input can be a constant operand. The case of two constant
755+
// operands should be handled by constant propagation.
756+
summary->set_in(1, summary->in(0).IsConstant()
757+
? Location::RequiresRegister()
758+
: Location::RegisterOrConstant(right()));
749759
summary->set_out(0, Location::RequiresRegister());
750760
return summary;
751761
}
752-
ASSERT(operation_cid() == kSmiCid);
753-
LocationSummary* summary = new (zone)
754-
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
755-
summary->set_in(0, Location::RegisterOrConstant(left()));
756-
// Only one input can be a constant operand. The case of two constant
757-
// operands should be handled by constant propagation.
758-
summary->set_in(1, summary->in(0).IsConstant()
759-
? Location::RequiresRegister()
760-
: Location::RegisterOrConstant(right()));
761-
summary->set_out(0, Location::RequiresRegister());
762-
return summary;
762+
UNREACHABLE();
763+
return NULL;
763764
}
764765

765766
Condition RelationalOpInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
766767
BranchLabels labels) {
767-
if ((operation_cid() == kSmiCid) || (operation_cid() == kMintCid)) {
768+
if (operation_cid() == kSmiCid || operation_cid() == kMintCid) {
768769
return EmitInt64ComparisonOp(compiler, *locs(), kind());
769770
} else {
770771
ASSERT(operation_cid() == kDoubleCid);
@@ -5158,24 +5159,30 @@ LocationSummary* BinaryInt64OpInstr::MakeLocationSummary(Zone* zone,
51585159
LocationSummary* summary = new (zone)
51595160
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
51605161
summary->set_in(0, Location::RequiresRegister());
5161-
summary->set_in(1, Location::RequiresRegister());
5162+
summary->set_in(1, Location::RegisterOrConstant(right()));
51625163
summary->set_out(0, Location::SameAsFirstInput());
51635164
return summary;
51645165
}
51655166

51665167
void BinaryInt64OpInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
5167-
const Register left = locs()->in(0).reg();
5168-
const Register right = locs()->in(1).reg();
5169-
const Register out = locs()->out(0).reg();
5170-
5171-
ASSERT(out == left);
5172-
51735168
Label* deopt = NULL;
51745169
if (CanDeoptimize()) {
51755170
deopt = compiler->AddDeoptStub(deopt_id(), ICData::kDeoptBinaryInt64Op);
51765171
}
5172+
const Location left = locs()->in(0);
5173+
const Location right = locs()->in(1);
5174+
const Location out = locs()->out(0);
5175+
ASSERT(out.reg() == left.reg());
51775176

5178-
EmitInt64Arithmetic(compiler, op_kind(), left, right, deopt);
5177+
if (right.IsConstant()) {
5178+
ConstantInstr* constant_instr = right.constant_instruction();
5179+
const int64_t value =
5180+
constant_instr->GetUnboxedSignedIntegerConstantValue();
5181+
EmitInt64Arithmetic(compiler, op_kind(), left.reg(), Immediate(value),
5182+
deopt);
5183+
} else {
5184+
EmitInt64Arithmetic(compiler, op_kind(), left.reg(), right.reg(), deopt);
5185+
}
51795186
}
51805187

51815188
LocationSummary* UnaryInt64OpInstr::MakeLocationSummary(Zone* zone,

0 commit comments

Comments
 (0)