[webkit-reviews] review granted: [Bug 224403] [JSC] B3 reduce-double-to-float should reduce only when constant double is canonical one to reduced float value : [Attachment 425678] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 224403: [JSC] B3 reduce-double-to-float should reduce only when constant
double is canonical one to reduced float value
https://bugs.webkit.org/show_bug.cgi?id=224403

Attachment 425678: Patch

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




--- Comment #3 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 425678
  --> https://bugs.webkit.org/attachment.cgi?id=425678
Patch

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

r=me with suggestion.

> Source/JavaScriptCore/b3/B3ReduceStrength.cpp:1690
> +	       // We cannot convert FloatToDouble(DoubleToFloat(value)) to
value, because double-to-float will trancate some range of double values into
one float.

/cannot convert/cannot convert some/

> Source/JavaScriptCore/b3/B3ReduceStrength.cpp:1696
>	       if (Value* constant =
m_value->child(0)->floatToDoubleConstant(m_proc)) {

Currently, both floatToDoubleConstant() and doubleToFloatConstant() do
unconditional conversions.  Since their results are always expected to be null
checked before use, I recommend either:
1. Make them only return a non-null result if the conversion is lossless, or
2. Add ASSERTs in them that the conversion is lossless.