[webkit-reviews] review granted: [Bug 125519] Make the different flavors of integer arithmetic more explicit, and don't rely on (possibly stale) results of the backwards propagator to decide integer arithmetic semantis : [Attachment 220466] the patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 125519: Make the different flavors of integer arithmetic more explicit, and
don't rely on (possibly stale) results of the backwards propagator to decide
integer arithmetic semantis
https://bugs.webkit.org/show_bug.cgi?id=125519

Attachment 220466: the patch
https://bugs.webkit.org/attachment.cgi?id=220466&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=220466&action=review


r=me

> Source/JavaScriptCore/ChangeLog:3
> +	   Make the different flavors of integer arithmetic more explicit, and
don't rely on (possibly stale) results of the backwards propagator to decide
integer arithmetic semantis

"semantics"

> Source/JavaScriptCore/dfg/DFGArithMode.h:38
> +    Unchecked, // Don't check anything and just do an integer operation.

I think this might be clearer as "Int" or "Hardware". "Unchecked" doesn't
really specify what *is* required, only what's *not* required.

> Source/JavaScriptCore/dfg/DFGArithMode.h:41
> +    DoOverflow // Even though the inputs are integers, up-convert them to
doubles and return a double.

I think this might be clearer as "Double" or "OverflowToDouble". At first, I
read "DoOverflow" as "allow the CPU to overflow in int space".

> Source/JavaScriptCore/dfg/DFGArithMode.h:45
> +inline bool doesOverflow(Arith::Mode mode)

I think this might be clearer as "isDouble" or "shouldOverflowToDouble".