<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body><span class="vcard"><a class="email" href="mailto:sbarati&#64;apple.com" title="Saam Barati &lt;sbarati&#64;apple.com&gt;"> <span class="fn">Saam Barati</span></a>
</span> changed
              <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>
        <br>
             <table border="1" cellspacing="0" cellpadding="8">
          <tr>
            <th>What</th>
            <th>Removed</th>
            <th>Added</th>
          </tr>

         <tr>
           <td style="text-align:right;">Attachment #286147 Flags</td>
           <td>review?
           </td>
           <td>review+
           </td>
         </tr></table>
      <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#c31">Comment # 31</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:sbarati&#64;apple.com" title="Saam Barati &lt;sbarati&#64;apple.com&gt;"> <span class="fn">Saam Barati</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=286147&amp;action=diff" name="attach_286147" title="Patch">attachment 286147</a> <a href="attachment.cgi?id=286147&amp;action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=286147&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=286147&amp;action=review</a>

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

<span class="quote">&gt; Source/JavaScriptCore/ChangeLog:9
&gt; +        a register to LHS or RHS if one of these operands are profiled as valid</span >

s/profiled/proven 

Profiled is not the correct requirement.

<span class="quote">&gt; Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3476
&gt; +    if (Generator::isLeftOperandValidConstant(leftOperand) || !Generator::isRightOperandValidConstant(rightOperand)) {</span >

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) &amp;&amp; Generator::isRightOperandValidConstant(rightOperand)));

<span class="quote">&gt; Source/JavaScriptCore/jit/JITArithmetic.cpp:741
&gt; +    if (Generator::isLeftOperandValidConstant(leftOperand) || !Generator::isRightOperandValidConstant(rightOperand))</span >

Ditto here. I'd do the same thing.

<span class="quote">&gt; Source/JavaScriptCore/jit/JITArithmetic.cpp:803
&gt;  </span >

I'd also add the assertion here.</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>