[webkit-reviews] review granted: [Bug 212069] Re-enable SharedArrayBuffer for JSC shell and Testers : [Attachment 413408] Patch

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 212069: Re-enable SharedArrayBuffer for JSC shell and Testers
https://bugs.webkit.org/show_bug.cgi?id=212069

Attachment 413408: Patch

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




--- Comment #14 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 413408
  --> https://bugs.webkit.org/attachment.cgi?id=413408
Patch

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

r=me with some nits, if you fix the style issues.

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2725
> +
> +	       if (m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex,
BadIndexingType)
> +		   ||
m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadConstantCache)
> +		   ||
m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadCache)
> +		   ||
m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadType))
> +		   return false;

Is there a reason to not include all exits (other than Uncountable and friends)
here?

> Source/JavaScriptCore/jsc.cpp:3102
> +    fprintf(stderr, "  --can-block-is-false	     Make main thread's
[[CanBlock]] false\n");

Nit: Maybe phrase this as "Make main thread's Atomics.wait throw"? Can block
may be a little obscure and I'm sure I'll forget in the future...

> Tools/Scripts/test262/Runner.pm:708
> +    if (grep $_ eq 'CanBlockIsFalse', @{$data->{flags}}) {
> +	   $featureFlags .= ' --can-block-is-false';
> +    }

Yikes, that seems expensive to do on every file...