Skip to content

Commit ec105c9

Browse files
ErikCorryGoogleErikCorryGoogle
committed
[VM] Unify filter methods in assemblers
[email protected] Inspired by https://chromereviews.googleplex.com/593797014/ (Vipunen StoreField write-barrier improvements) Next step is to move part of the write barrier to the end of the function. Bug: Change-Id: I4d49625c9cf5b3488e4633084790f4a018021866 Reviewed-on: https://dart-review.googlesource.com/31100 Reviewed-by: Martin Kustermann <[email protected]>
1 parent 0a9ee1c commit ec105c9

9 files changed

+199
-157
lines changed

‎assembler_arm.cc

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1532,33 +1532,25 @@ void Assembler::CompareObject(Register rn, const Object& object) {
15321532
}
15331533
}
15341534

1535-
// Preserves object and value registers.
1536-
void Assembler::StoreIntoObjectFilterNoSmi(Register object,
1537-
Register value,
1538-
Label* no_update) {
1539-
COMPILE_ASSERT((kNewObjectAlignmentOffset == kWordSize) &&
1540-
(kOldObjectAlignmentOffset == 0));
1541-
1542-
// Write-barrier triggers if the value is in the new space (has bit set) and
1543-
// the object is in the old space (has bit cleared).
1544-
// To check that, we compute value & ~object and skip the write barrier
1545-
// if the bit is not set. We can't destroy the object.
1546-
bic(IP, value, Operand(object));
1547-
tst(IP, Operand(kNewObjectAlignmentOffset));
1548-
b(no_update, EQ);
1549-
}
1550-
15511535
// Preserves object and value registers.
15521536
void Assembler::StoreIntoObjectFilter(Register object,
15531537
Register value,
1554-
Label* no_update) {
1538+
Label* label,
1539+
CanBeSmi value_can_be_smi,
1540+
BarrierFilterMode how_to_jump) {
1541+
COMPILE_ASSERT((kNewObjectAlignmentOffset == kWordSize) &&
1542+
(kOldObjectAlignmentOffset == 0));
15551543
// For the value we are only interested in the new/old bit and the tag bit.
15561544
// And the new bit with the tag bit. The resulting bit will be 0 for a Smi.
1557-
and_(IP, value, Operand(value, LSL, kObjectAlignmentLog2 - 1));
1558-
// And the result with the negated space bit of the object.
1559-
bic(IP, IP, Operand(object));
1545+
if (value_can_be_smi == kValueCanBeSmi) {
1546+
and_(IP, value, Operand(value, LSL, kObjectAlignmentLog2 - 1));
1547+
// And the result with the negated space bit of the object.
1548+
bic(IP, IP, Operand(object));
1549+
} else {
1550+
bic(IP, value, Operand(object));
1551+
}
15601552
tst(IP, Operand(kNewObjectAlignmentOffset));
1561-
b(no_update, EQ);
1553+
b(label, how_to_jump == kJumpToNoUpdate ? EQ : NE);
15621554
}
15631555

15641556
Register UseRegister(Register reg, RegList* used) {
@@ -1583,15 +1575,11 @@ Register AllocateRegister(RegList* used) {
15831575
void Assembler::StoreIntoObject(Register object,
15841576
const Address& dest,
15851577
Register value,
1586-
bool can_value_be_smi) {
1578+
CanBeSmi can_be_smi) {
15871579
ASSERT(object != value);
15881580
str(value, dest);
15891581
Label done;
1590-
if (can_value_be_smi) {
1591-
StoreIntoObjectFilter(object, value, &done);
1592-
} else {
1593-
StoreIntoObjectFilterNoSmi(object, value, &done);
1594-
}
1582+
StoreIntoObjectFilter(object, value, &done, can_be_smi, kJumpToNoUpdate);
15951583
// A store buffer update is required.
15961584
RegList regs = (1 << CODE_REG) | (1 << LR);
15971585
if (value != R0) {
@@ -1611,7 +1599,7 @@ void Assembler::StoreIntoObject(Register object,
16111599
void Assembler::StoreIntoObjectOffset(Register object,
16121600
int32_t offset,
16131601
Register value,
1614-
bool can_value_be_smi) {
1602+
CanBeSmi can_value_be_smi) {
16151603
int32_t ignored = 0;
16161604
if (Address::CanHoldStoreOffset(kWord, offset - kHeapObjectTag, &ignored)) {
16171605
StoreIntoObject(object, FieldAddress(object, offset), value,
@@ -1628,7 +1616,7 @@ void Assembler::StoreIntoObjectNoBarrier(Register object,
16281616
str(value, dest);
16291617
#if defined(DEBUG)
16301618
Label done;
1631-
StoreIntoObjectFilter(object, value, &done);
1619+
StoreIntoObjectFilter(object, value, &done, kValueCanBeSmi, kJumpToNoUpdate);
16321620
Stop("Store buffer update is required");
16331621
Bind(&done);
16341622
#endif // defined(DEBUG)
@@ -1694,8 +1682,10 @@ void Assembler::InitializeFieldsNoBarrier(Register object,
16941682
str(value_even, Address(begin, -2 * kWordSize), HI);
16951683
#if defined(DEBUG)
16961684
Label done;
1697-
StoreIntoObjectFilter(object, value_even, &done);
1698-
StoreIntoObjectFilter(object, value_odd, &done);
1685+
StoreIntoObjectFilter(object, value_even, &done, kValueCanBeSmi,
1686+
kJumpToNoUpdate);
1687+
StoreIntoObjectFilter(object, value_odd, &done, kValueCanBeSmi,
1688+
kJumpToNoUpdate);
16991689
Stop("Store buffer update is required");
17001690
Bind(&done);
17011691
#endif // defined(DEBUG)
@@ -1720,8 +1710,10 @@ void Assembler::InitializeFieldsNoBarrierUnrolled(Register object,
17201710
}
17211711
#if defined(DEBUG)
17221712
Label done;
1723-
StoreIntoObjectFilter(object, value_even, &done);
1724-
StoreIntoObjectFilter(object, value_odd, &done);
1713+
StoreIntoObjectFilter(object, value_even, &done, kValueCanBeSmi,
1714+
kJumpToNoUpdate);
1715+
StoreIntoObjectFilter(object, value_odd, &done, kValueCanBeSmi,
1716+
kJumpToNoUpdate);
17251717
Stop("Store buffer update is required");
17261718
Bind(&done);
17271719
#endif // defined(DEBUG)

‎assembler_arm.h

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -772,14 +772,19 @@ class Assembler : public ValueObject {
772772
void PushObject(const Object& object);
773773
void CompareObject(Register rn, const Object& object);
774774

775+
enum CanBeSmi {
776+
kValueIsNotSmi,
777+
kValueCanBeSmi,
778+
};
779+
775780
void StoreIntoObject(Register object, // Object we are storing into.
776781
const Address& dest, // Where we are storing into.
777782
Register value, // Value we are storing.
778-
bool can_value_be_smi = true);
783+
CanBeSmi can_value_be_smi = kValueCanBeSmi);
779784
void StoreIntoObjectOffset(Register object,
780785
int32_t offset,
781786
Register value,
782-
bool can_value_be_smi = true);
787+
CanBeSmi can_value_be_smi = kValueCanBeSmi);
783788

784789
void StoreIntoObjectNoBarrier(Register object,
785790
const Address& dest,
@@ -1227,12 +1232,21 @@ class Assembler : public ValueObject {
12271232
int32_t EncodeTstOffset(int32_t offset, int32_t inst);
12281233
int32_t DecodeTstOffset(int32_t inst);
12291234

1230-
void StoreIntoObjectFilter(Register object, Register value, Label* no_update);
1235+
enum BarrierFilterMode {
1236+
// Filter falls through into the barrier update code. Target label
1237+
// is a "after-store" label.
1238+
kJumpToNoUpdate,
12311239

1232-
// Shorter filtering sequence that assumes that value is not a smi.
1233-
void StoreIntoObjectFilterNoSmi(Register object,
1234-
Register value,
1235-
Label* no_update);
1240+
// Filter falls through to the "after-store" code. Target label
1241+
// is barrier update code label.
1242+
kJumpToBarrier,
1243+
};
1244+
1245+
void StoreIntoObjectFilter(Register object,
1246+
Register value,
1247+
Label* label,
1248+
CanBeSmi can_be_smi,
1249+
BarrierFilterMode barrier_filter_mode);
12361250

12371251
DISALLOW_ALLOCATION();
12381252
DISALLOW_COPY_AND_ASSIGN(Assembler);

‎assembler_arm64.cc

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -898,59 +898,56 @@ void Assembler::VRSqrts(VRegister vd, VRegister vn) {
898898
vmuls(vd, vd, VTMP);
899899
}
900900

901-
// Store into object.
902901
// Preserves object and value registers.
903-
void Assembler::StoreIntoObjectFilterNoSmi(Register object,
904-
Register value,
905-
Label* no_update) {
902+
void Assembler::StoreIntoObjectFilter(Register object,
903+
Register value,
904+
Label* label,
905+
CanBeSmi value_can_be_smi,
906+
BarrierFilterMode how_to_jump) {
906907
COMPILE_ASSERT((kNewObjectAlignmentOffset == kWordSize) &&
907908
(kOldObjectAlignmentOffset == 0));
908909

909910
// Write-barrier triggers if the value is in the new space (has bit set) and
910911
// the object is in the old space (has bit cleared).
911-
// To check that, we compute value & ~object and skip the write barrier
912-
// if the bit is not set. We can't destroy the object.
913-
bic(TMP, value, Operand(object));
914-
tbz(no_update, TMP, kNewObjectBitPosition);
915-
}
916-
917-
// Preserves object and value registers.
918-
void Assembler::StoreIntoObjectFilter(Register object,
919-
Register value,
920-
Label* no_update) {
921-
// For the value we are only interested in the new/old bit and the tag bit.
922-
// And the new bit with the tag bit. The resulting bit will be 0 for a Smi.
923-
and_(TMP, value, Operand(value, LSL, kNewObjectBitPosition));
924-
// And the result with the negated space bit of the object.
925-
bic(TMP, TMP, Operand(object));
926-
tbz(no_update, TMP, kNewObjectBitPosition);
912+
if (value_can_be_smi == kValueIsNotSmi) {
913+
// To check that, we compute value & ~object and skip the write barrier
914+
// if the bit is not set. We can't destroy the object.
915+
bic(TMP, value, Operand(object));
916+
} else {
917+
// For the value we are only interested in the new/old bit and the tag bit.
918+
// And the new bit with the tag bit. The resulting bit will be 0 for a Smi.
919+
and_(TMP, value, Operand(value, LSL, kNewObjectBitPosition));
920+
// And the result with the negated space bit of the object.
921+
bic(TMP, TMP, Operand(object));
922+
}
923+
if (how_to_jump == kJumpToNoUpdate) {
924+
tbz(label, TMP, kNewObjectBitPosition);
925+
} else {
926+
tbnz(label, TMP, kNewObjectBitPosition);
927+
}
927928
}
928929

929930
void Assembler::StoreIntoObjectOffset(Register object,
930931
int32_t offset,
931932
Register value,
932-
bool can_value_be_smi) {
933+
CanBeSmi value_can_be_smi) {
933934
if (Address::CanHoldOffset(offset - kHeapObjectTag)) {
934935
StoreIntoObject(object, FieldAddress(object, offset), value,
935-
can_value_be_smi);
936+
value_can_be_smi);
936937
} else {
937938
AddImmediate(TMP, object, offset - kHeapObjectTag);
938-
StoreIntoObject(object, Address(TMP), value, can_value_be_smi);
939+
StoreIntoObject(object, Address(TMP), value, value_can_be_smi);
939940
}
940941
}
941942

942943
void Assembler::StoreIntoObject(Register object,
943944
const Address& dest,
944945
Register value,
945-
bool can_value_be_smi) {
946+
CanBeSmi can_be_smi) {
946947
ASSERT(object != value);
947948
str(value, dest);
948949
Label done;
949-
if (can_value_be_smi) {
950-
StoreIntoObjectFilter(object, value, &done);
951-
} else {
952-
StoreIntoObjectFilterNoSmi(object, value, &done);
953-
}
950+
StoreIntoObjectFilter(object, value, &done, kValueCanBeSmi, kJumpToNoUpdate);
954951
// A store buffer update is required.
955952
if (value != R0) {
956953
// Preserve R0.
@@ -977,7 +974,7 @@ void Assembler::StoreIntoObjectNoBarrier(Register object,
977974
str(value, dest);
978975
#if defined(DEBUG)
979976
Label done;
980-
StoreIntoObjectFilter(object, value, &done);
977+
StoreIntoObjectFilter(object, value, &done, kValueCanBeSmi, kJumpToNoUpdate);
981978
Stop("Store buffer update is required");
982979
Bind(&done);
983980
#endif // defined(DEBUG)

‎assembler_arm64.h

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,15 +1439,20 @@ class Assembler : public ValueObject {
14391439
StoreQToOffset(src, base, offset - kHeapObjectTag);
14401440
}
14411441

1442+
enum CanBeSmi {
1443+
kValueIsNotSmi,
1444+
kValueCanBeSmi,
1445+
};
1446+
14421447
// Storing into an object.
14431448
void StoreIntoObject(Register object,
14441449
const Address& dest,
14451450
Register value,
1446-
bool can_value_be_smi = true);
1451+
CanBeSmi can_value_be_smi = kValueCanBeSmi);
14471452
void StoreIntoObjectOffset(Register object,
14481453
int32_t offset,
14491454
Register value,
1450-
bool can_value_be_smi = true);
1455+
CanBeSmi can_value_be_smi = kValueCanBeSmi);
14511456
void StoreIntoObjectNoBarrier(Register object,
14521457
const Address& dest,
14531458
Register value);
@@ -2206,12 +2211,21 @@ class Assembler : public ValueObject {
22062211
Emit(encoding);
22072212
}
22082213

2209-
void StoreIntoObjectFilter(Register object, Register value, Label* no_update);
2214+
enum BarrierFilterMode {
2215+
// Filter falls through into the barrier update code. Target label
2216+
// is a "after-store" label.
2217+
kJumpToNoUpdate,
22102218

2211-
// Shorter filtering sequence that assumes that value is not a smi.
2212-
void StoreIntoObjectFilterNoSmi(Register object,
2213-
Register value,
2214-
Label* no_update);
2219+
// Filter falls through to the "after-store" code. Target label
2220+
// is barrier update code label.
2221+
kJumpToBarrier,
2222+
};
2223+
2224+
void StoreIntoObjectFilter(Register object,
2225+
Register value,
2226+
Label* label,
2227+
CanBeSmi can_be_smi,
2228+
BarrierFilterMode barrier_filter_mode);
22152229

22162230
DISALLOW_ALLOCATION();
22172231
DISALLOW_COPY_AND_ASSIGN(Assembler);

‎assembler_dbc.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,12 @@ class Assembler : public ValueObject {
8080
static bool IsSafe(const Object& value) { return true; }
8181
static bool IsSafeSmi(const Object& value) { return false; }
8282

83-
// Bytecodes.
83+
enum CanBeSmi {
84+
kValueIsNotSmi,
85+
kValueCanBeSmi,
86+
};
87+
88+
// Bytecodes.
8489

8590
#define DECLARE_EMIT(Name, Signature, Fmt0, Fmt1, Fmt2) \
8691
void Name(PARAMS_##Signature);

0 commit comments

Comments
 (0)