[webkit-reviews] review granted: [Bug 214956] [JSC] Add B3::BottomTupleValue node : [Attachment 405542] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 29 20:09:11 PDT 2020


Keith Miller <keith_miller at apple.com> has granted Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 214956: [JSC] Add B3::BottomTupleValue node
https://bugs.webkit.org/show_bug.cgi?id=214956

Attachment 405542: Patch

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




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

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

r=me with nits.

> Source/JavaScriptCore/ChangeLog:10
> +	   While we support bottom values for usual types, we are not having
bottom value for tuple type. So when replaceWithBottom is called, we

Nit: we are not having bottom value => we do not have a bottom value.

> Source/JavaScriptCore/ChangeLog:11
> +	   failed to replace Patchpoint producing tuple with bottom values.

Nit: failed to replace Patchpoint producing tuple => fail to replace
Patchpoints producing tuples

> Source/JavaScriptCore/ChangeLog:13
> +	   This patch newly adds B3 BottomTupleValue, which is just generating
BottomValue for tuple. We can extend it to generate arbitrary constant

Nit: is just generating BottomValue => is just a BottomValue

> Source/JavaScriptCore/ChangeLog:14
> +	   tuple values, but for now, we just support bottom tuple value case.
We add a new node instead of generating patchpoint which generates bottom

Nit: value case => values.

> Source/JavaScriptCore/b3/B3Opcode.h:60
> +    // Tuple filled with zeros. This appears when Tuple Patchpoints are
replaced with Bottom values.
> +    BottomTuple,

Can we update the Extract comment to note that BottomTuple can emit tuples. I
would say we should update the HTML doc but I guess I never did that... whoops
��

> Source/JavaScriptCore/b3/testb3_7.cpp:1630
> +	   kind, kind, kind, kind, kind,
> +	   kind, kind, kind, kind, kind,
> +	   kind, kind, kind, kind, kind,
> +	   kind, kind, kind, kind, kind,
> +	   kind, kind, kind, kind, kind,
> +	   kind, kind, kind, kind, kind,
> +	   kind, kind, kind, kind, kind,
> +	   kind, kind, kind, kind, kind,
> +	   kind, kind, kind, kind, kind,
> +	   kind, kind, kind, kind, kind,

lol.


More information about the webkit-reviews mailing list