[JSC] Shrink the Math inline caches some more
Created attachment 289673 [details] Patch
Comment on attachment 289673 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=289673&action=review > Source/JavaScriptCore/ChangeLog:28 > + The inline cache average sizes on Sunspider change as follow: Very nice. > Source/JavaScriptCore/jit/JITArithmetic.cpp:762 > + ArithProfile* arithProfile = m_codeBlock->arithProfileForPC(currentInstruction); I think we should make JITMathIC have an ArithProfile& instead of an ArithProfile* since it's no longer an optional parameter. Maybe it's also worth having the field be on JOTMathIC instead of each individual generator? Also, there is code in the DFG/FTL that uses airthProfileForBytecodeOffset and passes in the result to the Generator's constructor, however, that function is around to return nullptr, so we should assert that it doesn't return nullptr or we should require the constructor to take ArithProfile& instead of ArithProfile*. What do you think?
Ah, typing on a phone: s/around/allowed s/JOTMathIC/JITMathIC
Created attachment 289738 [details] Patch
Turns out a null ArithProfile is a legit case. It happens if DFG produces a node from a different bytecode. For example, op_dec can be lowered to ArithSub. I did some clean up and moved ArithProfile to JITMathIC.
Created attachment 289763 [details] Patch
Comment on attachment 289763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=289763&action=review r=me > Source/JavaScriptCore/ChangeLog:15 > + of the generators. "the generators" => "the math IC" since you just changed this.
(In reply to comment #5) > Turns out a null ArithProfile is a legit case. It happens if DFG produces a > node from a different bytecode. For example, op_dec can be lowered to > ArithSub. Interesting. I didn't think about this situation. I wonder if it's worth it at all to have an ArithProfile for such cases, and just generate an empty IC to begin with? That may be worth it in the case where the decrement has a non-integer argument, but it will be worse in the case of an integer argument. What do you think? If it's worth it, you or me could later write a patch the just gives CodeBlock a Bag<ArithProfile> for such situations. This may be overkill though.
(In reply to comment #8) > (In reply to comment #5) > > Turns out a null ArithProfile is a legit case. It happens if DFG produces a > > node from a different bytecode. For example, op_dec can be lowered to > > ArithSub. > > Interesting. I didn't think about this situation. I wonder if it's worth it > at all to have an ArithProfile for such cases, and just generate an empty IC > to begin with? That may be worth it in the case where the decrement has a > non-integer argument, but it will be worse in the case of an integer > argument. What do you think? If it's worth it, you or me could later write a > patch the just gives CodeBlock a Bag<ArithProfile> for such situations. This > may be overkill though. I think you are right, creating an ArithProfile in those cases should yield better codegen. We could populate it from the children's prediction and update when the prediction were optimistic. Alternatively, we could start with the case that got us into UntypedUse and observe the fast cases on the fly.
Created attachment 289864 [details] Patch for landing
Comment on attachment 289864 [details] Patch for landing Clearing flags on attachment: 289864 Committed r206392: <http://trac.webkit.org/changeset/206392>
All reviewed patches have been landed. Closing bug.