[Webkit-unassigned] [Bug 160802] Register usage optimization in mathIC when LHS and RHS are constants isn't configured correctly

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 24 18:29:28 PDT 2016


Saam Barati <sbarati at apple.com> changed:

           What    |Removed                     |Added
 Attachment #286147|review?                     |review+
              Flags|                            |

--- Comment #31 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 286147
  --> https://bugs.webkit.org/attachment.cgi?id=286147

View in context: https://bugs.webkit.org/attachment.cgi?id=286147&action=review

r=me with suggestion to fix my incorrect analysis in my previous review

> Source/JavaScriptCore/ChangeLog:9
> +        a register to LHS or RHS if one of these operands are profiled as valid


Profiled is not the correct requirement.

> 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)));

> Source/JavaScriptCore/jit/JITArithmetic.cpp:741
> +    if (Generator::isLeftOperandValidConstant(leftOperand) || !Generator::isRightOperandValidConstant(rightOperand))

Ditto here. I'd do the same thing.

> Source/JavaScriptCore/jit/JITArithmetic.cpp:803

I'd also add the assertion here.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160825/82d0e446/attachment.html>

More information about the webkit-unassigned mailing list