Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (285151 => 285152)
--- trunk/Source/_javascript_Core/ChangeLog 2021-11-02 03:25:30 UTC (rev 285151)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-11-02 03:42:34 UTC (rev 285152)
@@ -1,3 +1,25 @@
+2021-11-01 Ross Kirsling <[email protected]>
+
+ [JSC][LLInt] Non-commutative binops are hard to reason about when operands are labelled in reverse
+ https://bugs.webkit.org/show_bug.cgi?id=232598
+
+ Reviewed by Saam Barati.
+
+ In offlineasm, `OP a, b, c` is `c = a OP b` but `OP a, b` is `b = b OP a`.
+
+ This can make identifiers like `left` and `right` quite confusing --
+ simple cases like `subd left, right` are already misleading, while OpDiv literally
+ passes its RHS to a macro as `left` and then checks `left` for division by zero.
+ It becomes difficult to keep this all in one's brain without rewriting it on paper.
+
+ This patch may not constitute a "complete solution", but it at least makes our naming honest:
+ 1. Use 3-argument syntax (as `left, right, result`) whenever possible.
+ 2. When not possible (e.g. because `bsubio` isn't flexible about its arguments or
+ because x86 doesn't have 3-argument shift operations), then say `rhs, lhs` explicitly.
+
+ * llint/LowLevelInterpreter32_64.asm:
+ * llint/LowLevelInterpreter64.asm:
+
2021-11-01 Yusuke Suzuki <[email protected]>
[JSC] LLIntCallee should have two replacements
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (285151 => 285152)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2021-11-02 03:25:30 UTC (rev 285151)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2021-11-02 03:42:34 UTC (rev 285152)
@@ -1118,7 +1118,7 @@
bineq t3, Int32Tag, .op2NotInt
updateBinaryArithProfile(size, opcodeStruct, ArithProfileIntInt, t5, t2)
get(m_dst, t2)
- integerOperationAndStore(t3, t1, t0, .slow, t2)
+ integerOperationAndStore(t3, t0, t1, .slow, t2)
dispatch()
.op1NotInt:
@@ -1135,7 +1135,7 @@
.op1NotIntReady:
get(m_dst, t1)
fii2d t0, t2, ft0
- doubleOperation(ft1, ft0)
+ doubleOperation(ft0, ft1, ft0)
stored ft0, [cfr, t1, 8]
dispatch()
@@ -1146,7 +1146,7 @@
get(m_dst, t2)
ci2ds t0, ft0
fii2d t1, t3, ft1
- doubleOperation(ft1, ft0)
+ doubleOperation(ft0, ft1, ft0)
stored ft0, [cfr, t2, 8]
dispatch()
@@ -1158,53 +1158,53 @@
macro binaryOp(opcodeName, opcodeStruct, integerOperation, doubleOperation)
binaryOpCustomStore(opcodeName, opcodeStruct,
- macro (int32Tag, left, right, slow, index)
- integerOperation(left, right, slow)
+ macro (int32Tag, lhs, rhs, slow, index)
+ integerOperation(lhs, rhs, slow)
storei int32Tag, TagOffset[cfr, index, 8]
- storei right, PayloadOffset[cfr, index, 8]
+ storei lhs, PayloadOffset[cfr, index, 8]
end,
doubleOperation)
end
binaryOp(add, OpAdd,
- macro (left, right, slow) baddio left, right, slow end,
- macro (left, right) addd left, right end)
+ macro (lhs, rhs, slow) baddio rhs, lhs, slow end,
+ macro (left, right, result) addd left, right, result end)
binaryOpCustomStore(mul, OpMul,
- macro (int32Tag, left, right, slow, index)
+ macro (int32Tag, lhs, rhs, slow, index)
const scratch = int32Tag # We know that we can reuse the int32Tag register since it has a constant.
- move right, scratch
- bmulio left, scratch, slow
+ move lhs, scratch
+ bmulio rhs, scratch, slow
btinz scratch, .done
- bilt left, 0, slow
- bilt right, 0, slow
+ bilt rhs, 0, slow
+ bilt lhs, 0, slow
.done:
storei Int32Tag, TagOffset[cfr, index, 8]
storei scratch, PayloadOffset[cfr, index, 8]
end,
- macro (left, right) muld left, right end)
+ macro (left, right, result) muld left, right, result end)
binaryOp(sub, OpSub,
- macro (left, right, slow) bsubio left, right, slow end,
- macro (left, right) subd left, right end)
+ macro (lhs, rhs, slow) bsubio rhs, lhs, slow end,
+ macro (left, right, result) subd left, right, result end)
binaryOpCustomStore(div, OpDiv,
- macro (int32Tag, left, right, slow, index)
- ci2ds left, ft0
- ci2ds right, ft1
+ macro (int32Tag, lhs, rhs, slow, index)
+ ci2ds rhs, ft0
+ ci2ds lhs, ft1
divd ft0, ft1
- bcd2i ft1, right, .notInt
+ bcd2i ft1, lhs, .notInt
storei int32Tag, TagOffset[cfr, index, 8]
- storei right, PayloadOffset[cfr, index, 8]
+ storei lhs, PayloadOffset[cfr, index, 8]
jmp .done
.notInt:
stored ft1, [cfr, index, 8]
.done:
end,
- macro (left, right) divd left, right end)
+ macro (left, right, result) divd left, right, result end)
llintOpWithReturn(op_unsigned, OpUnsigned, macro (size, get, dispatch, return)
@@ -1226,7 +1226,7 @@
loadConstantOrVariable2Reg(size, t0, t2, t0)
bineq t3, Int32Tag, .slow
bineq t2, Int32Tag, .slow
- operation(t1, t0)
+ operation(t0, t1)
return (t3, t0)
.slow:
@@ -1245,24 +1245,24 @@
bitOpProfiled(lshift, OpLshift,
- macro (left, right) lshifti left, right end)
+ macro (lhs, rhs) lshifti rhs, lhs end)
bitOp(rshift, OpRshift,
- macro (left, right) rshifti left, right end)
+ macro (lhs, rhs) rshifti rhs, lhs end)
bitOp(urshift, OpUrshift,
- macro (left, right) urshifti left, right end)
+ macro (lhs, rhs) urshifti rhs, lhs end)
bitOpProfiled(bitxor, OpBitxor,
- macro (left, right) xori left, right end)
+ macro (lhs, rhs) xori rhs, lhs end)
bitOpProfiled(bitand, OpBitand,
- macro (left, right) andi left, right end)
+ macro (lhs, rhs) andi rhs, lhs end)
bitOpProfiled(bitor, OpBitor,
- macro (left, right) ori left, right end)
+ macro (lhs, rhs) ori rhs, lhs end)
llintOpWithProfile(op_bitnot, OpBitnot, macro (size, get, dispatch, return)
get(m_operand, t0)
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (285151 => 285152)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2021-11-02 03:25:30 UTC (rev 285151)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2021-11-02 03:42:34 UTC (rev 285152)
@@ -1178,7 +1178,7 @@
bqb t0, numberTag, .op1NotInt
bqb t1, numberTag, .op2NotInt
get(m_dst, t2)
- integerOperationAndStore(t1, t0, .slow, t2)
+ integerOperationAndStore(t0, t1, .slow, t2)
updateBinaryArithProfile(size, opcodeStruct, ArithProfileIntInt, t5, t2)
dispatch()
@@ -1199,7 +1199,7 @@
get(m_dst, t2)
addq numberTag, t0
fq2d t0, ft0
- doubleOperation(ft1, ft0)
+ doubleOperation(ft0, ft1, ft0)
fd2q ft0, t0
subq numberTag, t0
storeq t0, [cfr, t2, 8]
@@ -1213,7 +1213,7 @@
ci2ds t0, ft0
addq numberTag, t1
fq2d t1, ft1
- doubleOperation(ft1, ft0)
+ doubleOperation(ft0, ft1, ft0)
fd2q ft0, t0
subq numberTag, t0
storeq t0, [cfr, t2, 8]
@@ -1227,17 +1227,17 @@
if X86_64 or X86_64_WIN
binaryOpCustomStore(div, OpDiv,
- macro (left, right, slow, index)
+ macro (lhs, rhs, slow, index)
# Assume t3 is scratchable.
- btiz left, slow
- bineq left, -1, .notNeg2TwoThe31DivByNeg1
- bieq right, -2147483648, .slow
+ btiz rhs, slow
+ bineq rhs, -1, .notNeg2TwoThe31DivByNeg1
+ bieq lhs, -2147483648, .slow
.notNeg2TwoThe31DivByNeg1:
- btinz right, .intOK
- bilt left, 0, slow
+ btinz lhs, .intOK
+ bilt rhs, 0, slow
.intOK:
- move left, t3
- move right, t0
+ move rhs, t3
+ move lhs, t0
cdqi
idivi t3
btinz t1, slow
@@ -1244,7 +1244,7 @@
orq numberTag, t0
storeq t0, [cfr, index, 8]
end,
- macro (left, right) divd left, right end)
+ macro (left, right, result) divd left, right, result end)
else
slowPathOp(div)
end
@@ -1251,38 +1251,38 @@
binaryOpCustomStore(mul, OpMul,
- macro (left, right, slow, index)
+ macro (lhs, rhs, slow, index)
# Assume t3 is scratchable.
- move right, t3
- bmulio left, t3, slow
+ move lhs, t3
+ bmulio rhs, t3, slow
btinz t3, .done
- bilt left, 0, slow
- bilt right, 0, slow
+ bilt rhs, 0, slow
+ bilt lhs, 0, slow
.done:
orq numberTag, t3
storeq t3, [cfr, index, 8]
end,
- macro (left, right) muld left, right end)
+ macro (left, right, result) muld left, right, result end)
macro binaryOp(opcodeName, opcodeStruct, integerOperation, doubleOperation)
binaryOpCustomStore(opcodeName, opcodeStruct,
- macro (left, right, slow, index)
- integerOperation(left, right, slow)
- orq numberTag, right
- storeq right, [cfr, index, 8]
+ macro (lhs, rhs, slow, index)
+ integerOperation(lhs, rhs, slow)
+ orq numberTag, lhs
+ storeq lhs, [cfr, index, 8]
end,
doubleOperation)
end
binaryOp(add, OpAdd,
- macro (left, right, slow) baddio left, right, slow end,
- macro (left, right) addd left, right end)
+ macro (lhs, rhs, slow) baddio rhs, lhs, slow end,
+ macro (left, right, result) addd left, right, result end)
binaryOp(sub, OpSub,
- macro (left, right, slow) bsubio left, right, slow end,
- macro (left, right) subd left, right end)
+ macro (lhs, rhs, slow) bsubio rhs, lhs, slow end,
+ macro (left, right, result) subd left, right, result end)
llintOpWithReturn(op_unsigned, OpUnsigned, macro (size, get, dispatch, return)
@@ -1304,7 +1304,7 @@
loadConstantOrVariable(size, t2, t0)
bqb t0, numberTag, .slow
bqb t1, numberTag, .slow
- operation(t1, t0)
+ operation(t0, t1)
orq numberTag, t0
return(t0)
@@ -1323,24 +1323,24 @@
end
bitOpProfiled(lshift, OpLshift,
- macro (left, right) lshifti left, right end)
+ macro (lhs, rhs) lshifti rhs, lhs end)
bitOpProfiled(rshift, OpRshift,
- macro (left, right) rshifti left, right end)
+ macro (lhs, rhs) rshifti rhs, lhs end)
bitOp(urshift, OpUrshift,
- macro (left, right) urshifti left, right end)
+ macro (lhs, rhs) urshifti rhs, lhs end)
bitOpProfiled(bitand, OpBitand,
- macro (left, right) andi left, right end)
+ macro (lhs, rhs) andi rhs, lhs end)
bitOpProfiled(bitor, OpBitor,
- macro (left, right) ori left, right end)
+ macro (lhs, rhs) ori rhs, lhs end)
bitOpProfiled(bitxor, OpBitxor,
- macro (left, right) xori left, right end)
+ macro (lhs, rhs) xori rhs, lhs end)
llintOpWithProfile(op_bitnot, OpBitnot, macro (size, get, dispatch, return)
get(m_operand, t0)
