[webkit-reviews] review canceled: [Bug 201373] [JSC] Merge op_check_traps into op_enter and op_loop_hint : [Attachment 377785] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has canceled Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 201373: [JSC] Merge op_check_traps into op_enter and op_loop_hint
https://bugs.webkit.org/show_bug.cgi?id=201373

Attachment 377785: Patch

https://bugs.webkit.org/attachment.cgi?id=377785&action=review




--- Comment #7 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 377785
  --> https://bugs.webkit.org/attachment.cgi?id=377785
Patch

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

>> Source/JavaScriptCore/jit/JITOpcodes.cpp:1034
>> +	addSlowCase(branchTest8(NonZero,
AbsoluteAddress(m_vm->needTrapHandlingAddress())));
> 
> I think we want to check traps before we check for optimization.  This is
because if we OSR exit due to a trap, we want to handle the trap first. 
Otherwise, there's a chance we'll be caught in a optimize + OSR exit loop.  In
practice, this might not happen, but I think it is more correct to check traps
first.

OK, fixed.

>> Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:1019
>> +	addSlowCase(branchTest8(NonZero,
AbsoluteAddress(m_vm->needTrapHandlingAddress())));
> 
> Ditto here.  Let's check traps first before checking for optimizing.

Fixed.

>> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:1678
>> +	btpnz VM::m_traps + VMTraps::m_needTrapHandling[t1], .handleTraps
> 
> Ditto here.  Let's check traps first.  This will be meaningful if we do OSR
exit to LLInt later.

Fixed.