Modified: trunk/Source/_javascript_Core/ChangeLog (279592 => 279593)
--- trunk/Source/_javascript_Core/ChangeLog 2021-07-06 17:26:48 UTC (rev 279592)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-07-06 17:29:30 UTC (rev 279593)
@@ -1,3 +1,82 @@
+2021-07-06 Yijia Huang <yijia_hu...@apple.com>
+
+ Fix instruction check failure of UBFX and SBFIZ in testb3 due to the speculative fix in bug 227554
+ https://bugs.webkit.org/show_bug.cgi?id=227563
+
+ Reviewed by Filip Pizlo.
+
+ This patch includes two modifications to resolve rdar://79978150:
+ 1. Fix the bug caused by the patch introducing BFI.
+ 2. Discard the corresponding speculative fix in https://bugs.webkit.org/show_bug.cgi?id=227554.
+
+ The previous patch, added bit field insert (BFI) to AIR opcode, causes the Gmail page
+ hanging issue on Safari. The root cause is the incorrect definition of register role in
+ BFI's AIR opcode. Since BFI inserts a bit field at the low end of the destination register
+ while keeping the high bit unchanged, the destination register should have both roles of
+ use and define simultaneously, which is not (missing use) in the previous patch.
+
+ This will result in the loss of preserving the value of the destination register,
+ which does happen when browsing the Gmail page on Safari.
+
+ // B3 IR snippets from Gmail
+ Int32 b@23 = Add(b@104, b@111, D@100)
+ ...
+ Int32 b@55 = Const32(65535, D@50)
+ Int32 b@137 = BitAnd(b@118, $65535(b@55), D@160)
+ Int32 b@168 = Const32(16, D@40)
+ Int32 b@141 = Shl(b@137, $16(b@168), D@163)
+ Int32 b@143 = BitAnd(b@23, $65535(b@55), D@166)
+ Int32 b@144 = BitOr(b@141, b@143, D@169)
+
+ The pattern of BFI is d = ((n & mask1) << lsb) | (d & mask2). So, it is obvious that
+ BFI can be utilized in b@144 where the d is b@23.
+
+ // Incorrect AIR opcode of BFI
+ arm64: InsertBitField32 U:G:32, U:G:32, U:G:32, ZD:G:32
+ Tmp, Imm, Imm, Tmp
+
+ // Air w/o use role
+ Add32 %x3, %x7, %x7, b@23
+ ...
+ InsertBitField32 %x3, $16, $16, %x4, b@144
+
+ // Generated code w/o use role
+ add w7, w3, w7
+ ...
+ bfi w4, w3, #16, #16
+
+ In Air, the added value is stored in the w7. But the value is not preserved after
+ lowering with BFI. To fix this, the use role should be enabled for the destination
+ register.
+
+ // Correnct AIR opcode of BFI
+ arm64: InsertBitField32 U:G:32, U:G:32, U:G:32, UZD:G:32
+ Tmp, Imm, Imm, Tmp
+
+ // Air w/ use role
+ Add32 %x3, %x7, %x7, b@23
+ ...
+ Move32 %x7, %x4, b@144
+ InsertBitField32 %x3, $16, $16, %x4, b@144
+
+ // Generated code w/ use role
+ add w7, w3, w7
+ ...
+ ubfx x4, x7, #0, #32
+ bfi w4, w3, #16, #16
+
+ In addition, BFXIL, which has pattern d = ((n >> lsb) & mask1) | (d & mask2), also needs
+ the similar update.
+
+ * b3/B3LowerToAir.cpp:
+ * b3/air/AirOpcode.opcodes:
+ * b3/testb3_2.cpp:
+ (testInsertBitField32):
+ (testInsertBitField64):
+ (testExtractInsertBitfieldAtLowEnd32):
+ (testExtractInsertBitfieldAtLowEnd64):
+ * runtime/OptionsList.h:
+
2021-07-04 Robin Morisset <rmoris...@apple.com>
ActiveScratchBufferScope should take the buffer as argument
Modified: trunk/Source/_javascript_Core/b3/air/AirOpcode.opcodes (279592 => 279593)
--- trunk/Source/_javascript_Core/b3/air/AirOpcode.opcodes 2021-07-06 17:26:48 UTC (rev 279592)
+++ trunk/Source/_javascript_Core/b3/air/AirOpcode.opcodes 2021-07-06 17:29:30 UTC (rev 279593)
@@ -824,10 +824,10 @@
arm64: InsertUnsignedBitfieldInZero64 U:G:64, U:G:32, U:G:32, D:G:64
Tmp, Imm, Imm, Tmp
-arm64: InsertBitField32 U:G:32, U:G:32, U:G:32, ZD:G:32
+arm64: InsertBitField32 U:G:32, U:G:32, U:G:32, UZD:G:32
Tmp, Imm, Imm, Tmp
-arm64: InsertBitField64 U:G:64, U:G:32, U:G:32, D:G:64
+arm64: InsertBitField64 U:G:64, U:G:32, U:G:32, UD:G:64
Tmp, Imm, Imm, Tmp
arm64: ClearBitField32 U:G:32, U:G:32, ZD:G:32
@@ -848,10 +848,10 @@
arm64: OrNot64 U:G:64, U:G:64, D:G:64
Tmp, Tmp, Tmp
-arm64: ExtractInsertBitfieldAtLowEnd32 U:G:32, U:G:32, U:G:32, ZD:G:32
+arm64: ExtractInsertBitfieldAtLowEnd32 U:G:32, U:G:32, U:G:32, UZD:G:32
Tmp, Imm, Imm, Tmp
-arm64: ExtractInsertBitfieldAtLowEnd64 U:G:64, U:G:32, U:G:32, D:G:64
+arm64: ExtractInsertBitfieldAtLowEnd64 U:G:64, U:G:32, U:G:32, UD:G:64
Tmp, Imm, Imm, Tmp
arm64: InsertSignedBitfieldInZero32 U:G:32, U:G:32, U:G:32, ZD:G:32
Modified: trunk/Source/_javascript_Core/b3/testb3_2.cpp (279592 => 279593)
--- trunk/Source/_javascript_Core/b3/testb3_2.cpp 2021-07-06 17:26:48 UTC (rev 279592)
+++ trunk/Source/_javascript_Core/b3/testb3_2.cpp 2021-07-06 17:29:30 UTC (rev 279593)
@@ -3160,8 +3160,6 @@
void testInsertBitField32()
{
- if (!JSC::Options::useBFI())
- return;
if (JSC::Options::defaultB3OptLevel() < 2)
return;
uint32_t d = 0xf0f0f0f0;
@@ -3199,6 +3197,13 @@
return invoke<uint32_t>(*code, d, n);
};
+ for (size_t i = 0; i < lsbs.size(); ++i) {
+ uint32_t lsb = lsbs.at(i);
+ uint32_t mask1 = (1U << widths.at(i)) - 1U;
+ uint32_t mask2 = ~(mask1 << lsb);
+ CHECK(test1(lsb, mask1, mask2) == (((n & mask1) << lsb) | (d & mask2)));
+ }
+
// Test Pattern: d = (d & mask2) | ((n & mask1) << lsb)
// Where: mask1 = ((1 << width) - 1)
// mask2 = ~(mask1 << lsb)
@@ -3233,15 +3238,70 @@
uint32_t lsb = lsbs.at(i);
uint32_t mask1 = (1U << widths.at(i)) - 1U;
uint32_t mask2 = ~(mask1 << lsb);
- CHECK(test1(lsb, mask1, mask2) == (((n & mask1) << lsb) | (d & mask2)));
CHECK(test2(lsb, mask1, mask2) == ((d & mask2) | ((n & mask1) << lsb)));
}
+
+ // Test use role on destination register of BFI
+ uint32_t dA = 0x0000f0f0;
+ uint32_t dB = 0xf0f00000;
+
+ auto test3 = [&] (uint32_t lsb, uint32_t mask1, uint32_t mask2) -> uint32_t {
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+
+ Value* dAValue = root->appendNew<Value>(
+ proc, Trunc, Origin(),
+ root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
+ Value* dBValue = root->appendNew<Value>(
+ proc, Trunc, Origin(),
+ root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1));
+ Value* nValue = root->appendNew<Value>(
+ proc, Trunc, Origin(),
+ root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR2));
+ Value* lsbValue = root->appendNew<Const32Value>(proc, Origin(), lsb);
+ Value* maskValue1 = root->appendNew<Const32Value>(proc, Origin(), mask1);
+ Value* maskValue2 = root->appendNew<Const32Value>(proc, Origin(), mask2);
+
+ // d = dA + dB
+ Value* dValue = root->appendNew<Value>(proc, Add, Origin(), dAValue, dBValue);
+
+ // d = (d & mask2) | ((n & mask1) << lsb)
+ Value* leftAndValue = root->appendNew<Value>(proc, BitAnd, Origin(), nValue, maskValue1);
+ Value* shiftValue = root->appendNew<Value>(proc, Shl, Origin(), leftAndValue, lsbValue);
+ Value* rightAndValue = root->appendNew<Value>(proc, BitAnd, Origin(), dValue, maskValue2);
+ Value* orValue = root->appendNew<Value>(proc, BitOr, Origin(), shiftValue, rightAndValue);
+
+ // v3 = ((mask1 + mask2) + dB) + dA
+ Value* value1 = root->appendNew<Value>(proc, Add, Origin(), maskValue1, maskValue2);
+ Value* value2 = root->appendNew<Value>(proc, Add, Origin(), value1, dBValue);
+ Value* value3 = root->appendNew<Value>(proc, Add, Origin(), value2, dAValue);
+
+ root->appendNewControlValue(
+ proc, Return, Origin(),
+ root->appendNew<Value>(proc, Add, Origin(), value3, orValue));
+
+ auto code = compileProc(proc);
+ if (isARM64())
+ checkUsesInstruction(*code, "bfi");
+ return invoke<uint32_t>(*code, dA, dB, n);
+ };
+
+ for (size_t i = 0; i < lsbs.size(); ++i) {
+ uint32_t lsb = lsbs.at(i);
+ uint32_t mask1 = (1U << widths.at(i)) - 1U;
+ uint32_t mask2 = ~(mask1 << lsb);
+
+ uint32_t lhs3 = test3(lsb, mask1, mask2);
+ uint32_t dv = dA + dB;
+ dv = (dv & mask2) | ((n & mask1) << lsb);
+ uint32_t v3 = ((mask1 + mask2) + dB) + dA;
+ uint32_t rhs3 = v3 + dv;
+ CHECK(lhs3 == rhs3);
+ }
}
void testInsertBitField64()
{
- if (!JSC::Options::useBFI())
- return;
if (JSC::Options::defaultB3OptLevel() < 2)
return;
uint64_t d = 0xf0f0f0f0f0f0f0f0;
@@ -3308,6 +3368,58 @@
CHECK(test1(lsb, mask1, mask2) == (((n & mask1) << lsb) | (d & mask2)));
CHECK(test2(lsb, mask1, mask2) == ((d & mask2) | ((n & mask1) << lsb)));
}
+
+ // Test use role on destination register of BFI
+ uint64_t dA = 0x00000000f0f0f0f0;
+ uint64_t dB = 0xf0f0f0f000000000;
+
+ auto test3 = [&] (uint64_t lsb, uint64_t mask1, uint64_t mask2) -> uint64_t {
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+
+ Value* dAValue = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+ Value* dBValue = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
+ Value* nValue = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR2);
+ Value* lsbValue = root->appendNew<Const32Value>(proc, Origin(), lsb);
+ Value* maskValue1 = root->appendNew<Const64Value>(proc, Origin(), mask1);
+ Value* maskValue2 = root->appendNew<Const64Value>(proc, Origin(), mask2);
+
+ // d = dA + dB
+ Value* dValue = root->appendNew<Value>(proc, Add, Origin(), dAValue, dBValue);
+
+ // d = (d & mask2) | ((n & mask1) << lsb)
+ Value* leftAndValue = root->appendNew<Value>(proc, BitAnd, Origin(), nValue, maskValue1);
+ Value* shiftValue = root->appendNew<Value>(proc, Shl, Origin(), leftAndValue, lsbValue);
+ Value* rightAndValue = root->appendNew<Value>(proc, BitAnd, Origin(), dValue, maskValue2);
+ Value* orValue = root->appendNew<Value>(proc, BitOr, Origin(), shiftValue, rightAndValue);
+
+ // v3 = ((mask1 + mask2) + dB) + dA
+ Value* value1 = root->appendNew<Value>(proc, Add, Origin(), maskValue1, maskValue2);
+ Value* value2 = root->appendNew<Value>(proc, Add, Origin(), value1, dBValue);
+ Value* value3 = root->appendNew<Value>(proc, Add, Origin(), value2, dAValue);
+
+ root->appendNewControlValue(
+ proc, Return, Origin(),
+ root->appendNew<Value>(proc, Add, Origin(), value3, orValue));
+
+ auto code = compileProc(proc);
+ if (isARM64())
+ checkUsesInstruction(*code, "bfi");
+ return invoke<uint64_t>(*code, dA, dB, n);
+ };
+
+ for (size_t i = 0; i < lsbs.size(); ++i) {
+ uint64_t lsb = lsbs.at(i);
+ uint64_t mask1 = (1ULL << widths.at(i)) - 1ULL;
+ uint64_t mask2 = ~(mask1 << lsb);
+
+ uint64_t lhs3 = test3(lsb, mask1, mask2);
+ uint64_t dv = dA + dB;
+ dv = (dv & mask2) | ((n & mask1) << lsb);
+ uint64_t v3 = ((mask1 + mask2) + dB) + dA;
+ uint64_t rhs3 = v3 + dv;
+ CHECK(lhs3 == rhs3);
+ }
}
void testExtractInsertBitfieldAtLowEnd32()
@@ -3392,6 +3504,64 @@
uint32_t mask2 = ~(mask1 >> lsb);
CHECK(test2(lsb, mask1, mask2) == (((n & mask1) >> lsb) | (d & mask2)));
}
+
+ // Test use role on destination register of BFXIL
+ uint32_t dA = 0x0000f0f0;
+ uint32_t dB = 0xf0f00000;
+
+ auto test3 = [&] (uint32_t lsb, uint32_t mask1, uint32_t mask2) -> uint32_t {
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+
+ Value* dAValue = root->appendNew<Value>(
+ proc, Trunc, Origin(),
+ root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
+ Value* dBValue = root->appendNew<Value>(
+ proc, Trunc, Origin(),
+ root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1));
+ Value* nValue = root->appendNew<Value>(
+ proc, Trunc, Origin(),
+ root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR2));
+ Value* lsbValue = root->appendNew<Const32Value>(proc, Origin(), lsb);
+ Value* maskValue1 = root->appendNew<Const32Value>(proc, Origin(), mask1);
+ Value* maskValue2 = root->appendNew<Const32Value>(proc, Origin(), mask2);
+
+ // d = dA + dB
+ Value* dValue = root->appendNew<Value>(proc, Add, Origin(), dAValue, dBValue);
+
+ // d = d = ((n >> lsb) & mask1) | (d & mask2)
+ Value* shiftValue = root->appendNew<Value>(proc, ZShr, Origin(), nValue, lsbValue);
+ Value* leftAndValue = root->appendNew<Value>(proc, BitAnd, Origin(), shiftValue, maskValue1);
+ Value* rightAndValue = root->appendNew<Value>(proc, BitAnd, Origin(), dValue, maskValue2);
+ Value* orValue = root->appendNew<Value>(proc, BitOr, Origin(), leftAndValue, rightAndValue);
+
+ // v3 = ((mask1 + mask2) + dB) + dA
+ Value* value1 = root->appendNew<Value>(proc, Add, Origin(), maskValue1, maskValue2);
+ Value* value2 = root->appendNew<Value>(proc, Add, Origin(), value1, dBValue);
+ Value* value3 = root->appendNew<Value>(proc, Add, Origin(), value2, dAValue);
+
+ root->appendNewControlValue(
+ proc, Return, Origin(),
+ root->appendNew<Value>(proc, Add, Origin(), value3, orValue));
+
+ auto code = compileProc(proc);
+ if (isARM64())
+ checkUsesInstruction(*code, "bfxil");
+ return invoke<uint32_t>(*code, dA, dB, n);
+ };
+
+ for (size_t i = 0; i < lsbs.size(); ++i) {
+ uint32_t lsb = lsbs.at(i);
+ uint32_t mask1 = (1U << widths.at(i)) - 1U;
+ uint32_t mask2 = ~mask1;
+
+ uint32_t lhs3 = test3(lsb, mask1, mask2);
+ uint32_t dv = dA + dB;
+ dv = ((n >> lsb) & mask1) | (dv & mask2);
+ uint32_t v3 = ((mask1 + mask2) + dB) + dA;
+ uint32_t rhs3 = v3 + dv;
+ CHECK(lhs3 == rhs3);
+ }
}
void testExtractInsertBitfieldAtLowEnd64()
@@ -3468,6 +3638,58 @@
uint64_t mask2 = ~(mask1 >> lsb);
CHECK(test2(lsb, mask1, mask2) == (((n & mask1) >> lsb) | (d & mask2)));
}
+
+ // Test use role on destination register of BFXIL
+ uint64_t dA = 0x00000000f0f0f0f0;
+ uint64_t dB = 0xf0f0f0f000000000;
+
+ auto test3 = [&] (uint64_t lsb, uint64_t mask1, uint64_t mask2) -> uint64_t {
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+
+ Value* dAValue = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+ Value* dBValue = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
+ Value* nValue = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR2);
+ Value* lsbValue = root->appendNew<Const32Value>(proc, Origin(), lsb);
+ Value* maskValue1 = root->appendNew<Const64Value>(proc, Origin(), mask1);
+ Value* maskValue2 = root->appendNew<Const64Value>(proc, Origin(), mask2);
+
+ // d = dA + dB
+ Value* dValue = root->appendNew<Value>(proc, Add, Origin(), dAValue, dBValue);
+
+ // d = d = ((n >> lsb) & mask1) | (d & mask2)
+ Value* shiftValue = root->appendNew<Value>(proc, ZShr, Origin(), nValue, lsbValue);
+ Value* leftAndValue = root->appendNew<Value>(proc, BitAnd, Origin(), shiftValue, maskValue1);
+ Value* rightAndValue = root->appendNew<Value>(proc, BitAnd, Origin(), dValue, maskValue2);
+ Value* orValue = root->appendNew<Value>(proc, BitOr, Origin(), leftAndValue, rightAndValue);
+
+ // v3 = ((mask1 + mask2) + dB) + dA
+ Value* value1 = root->appendNew<Value>(proc, Add, Origin(), maskValue1, maskValue2);
+ Value* value2 = root->appendNew<Value>(proc, Add, Origin(), value1, dBValue);
+ Value* value3 = root->appendNew<Value>(proc, Add, Origin(), value2, dAValue);
+
+ root->appendNewControlValue(
+ proc, Return, Origin(),
+ root->appendNew<Value>(proc, Add, Origin(), value3, orValue));
+
+ auto code = compileProc(proc);
+ if (isARM64())
+ checkUsesInstruction(*code, "bfxil");
+ return invoke<uint64_t>(*code, dA, dB, n);
+ };
+
+ for (size_t i = 0; i < lsbs.size(); ++i) {
+ uint64_t lsb = lsbs.at(i);
+ uint64_t mask1 = (1ULL << widths.at(i)) - 1ULL;
+ uint64_t mask2 = ~mask1;
+
+ uint64_t lhs3 = test3(lsb, mask1, mask2);
+ uint64_t dv = dA + dB;
+ dv = ((n >> lsb) & mask1) | (dv & mask2);
+ uint64_t v3 = ((mask1 + mask2) + dB) + dA;
+ uint64_t rhs3 = v3 + dv;
+ CHECK(lhs3 == rhs3);
+ }
}
void testBIC32()
