[webkit-reviews] review denied: [Bug 192663] [BigInt] Add ValueBitRShift into DFG : [Attachment 359607] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has denied Caio Lima <ticaiolima at gmail.com>'s
request for review:
Bug 192663: [BigInt] Add ValueBitRShift into DFG
https://bugs.webkit.org/show_bug.cgi?id=192663

Attachment 359607: Patch

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




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

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

mostly LGTM, but you have a doesGC bug

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:400
> +    case ValueBitRShift:

you should abstract the code to still do constant folding. Just b/c we're
UntypedUse should not mean we can't constant fold. It'd also be nice to add
constant folding for BigInts, but perhaps that can be done in a followup patch,
since it's not obvious what to do, since we'd want to allocate a JSCell

> Source/JavaScriptCore/dfg/DFGDoesGC.cpp:103
> +    case ValueBitRShift:

should return true for BigIntUse