<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Register usage optimization in mathIC when LHS and RHS are constants isn't configured correctly"
href="https://bugs.webkit.org/show_bug.cgi?id=160802#c32">Comment # 32</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Register usage optimization in mathIC when LHS and RHS are constants isn't configured correctly"
href="https://bugs.webkit.org/show_bug.cgi?id=160802">bug 160802</a>
from <span class="vcard"><a class="email" href="mailto:ticaiolima@gmail.com" title="Caio Lima <ticaiolima@gmail.com>"> <span class="fn">Caio Lima</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=286147&action=diff" name="attach_286147" title="Patch">attachment 286147</a> <a href="attachment.cgi?id=286147&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=286147&action=review">https://bugs.webkit.org/attachment.cgi?id=286147&action=review</a>
<span class="quote">>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3476
>> + if (Generator::isLeftOperandValidConstant(leftOperand) || !Generator::isRightOperandValidConstant(rightOperand)) {
>
> Sorry, I was totally wrong about this code. We don't need to check Generator::isLeftOperandValidConstant(leftOperand) here since
> the code above makes sure that not both operands can be constant. Sorry for incorrectly suggesting that before.
> Instead, what I'd do is just have an assertion above like so (by the other related assertion):
> ASSERT(!(Generator::isLeftOperandValidConstant(leftOperand) && Generator::isRightOperandValidConstant(rightOperand)));</span >
Actually, IMHO I think it is correct. The code previously is just considering "isInt32Constant" and coincidently our current JITMathICs are just considering Int32Constant as possible constant operand. However, one of them can potentially consider a rule with "isNumberConstant" or even RHS or LHS as always valid constant and "(left|right)Child->isInt32Constant" is not considering it. I think this design better because we leave the valid constant rule in JITBlahGenerator responsibility. Does it make sense to you?
<span class="quote">>> Source/JavaScriptCore/jit/JITArithmetic.cpp:741
>> + if (Generator::isLeftOperandValidConstant(leftOperand) || !Generator::isRightOperandValidConstant(rightOperand))
>
> Ditto here. I'd do the same thing.</span >
Ditto above.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>