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

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=160802

Saam Barati <sbarati at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #286049|review?                     |review-
              Flags|                            |

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

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

> Source/JavaScriptCore/ChangeLog:10
> +        We created in this Patch isLeftOperandValidConstant(SnippetOperand) and
> +        isRighttOperandValidConstant(SnippetOperand) in JIT*Generators because
> +        JSC::DFG::SpeculativeJIT::compileMathIC also need to check

This changelog isn't very clear. Can you rewrite this to more clearly state that this patch fixes
a broken optimization where we would always populate the lhs/rhs registers even though
the snippet could handle either lhs/rhs not being populated because it's a constant.

> Source/JavaScriptCore/ChangeLog:13
> +        Generator Object. The old version was always returning true
> +        because mathIC->generator was a fresh allocated object.

Was returning true to what? Do you mean each (lhs/rhs)IsvalidConstant were returning false before?

> Source/JavaScriptCore/jit/JITArithmetic.cpp:741
> +    if (!Generator::isLeftOperandValidConstant(leftOperand))
>          emitGetVirtualRegister(op1, leftRegs);
> -    if (!mathIC->isRightOperandValidConstant())
> +    if (!Generator::isRightOperandValidConstant(rightOperand))

This doesn't seem correct. This allows for both lhs and rhs to be constant.
I think you want the second condition to be
if (Generator::isLeftOperandValidConstant(leftOperand) || !Generator::isRightOperandValidConstant(rightOperand))

-- 
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/20160816/8a27e743/attachment.html>