[JSC][32bit] Fix undefined behavior causing misscompilation with clang 13 on ARM
Created attachment 447369 [details] Patch
Created attachment 447370 [details] Patch
Created attachment 447373 [details] Patch
Comment on attachment 447373 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447373&action=review > Source/JavaScriptCore/runtime/MathCommon.h:224 > { > - // Note: Strictly speaking this is an undefined behavior. > - return static_cast<int32_t>(value) == value; > + return canBeInt32(value) && !(!value && std::signbit(value)); // false for -0.0 > } This needs to be super fast. I think we should inline canBeInt32's code here, and use !asInt32 instead of !value.
Created attachment 447451 [details] Patch
Comment on attachment 447451 [details] Patch r=me
Committed r287235 (245395@main): <https://commits.webkit.org/245395@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447451 [details].
<rdar://problem/86678800>
I prefer std::isfinite for checks like this.
(In reply to Darin Adler from comment #9) > I prefer std::isfinite for checks like this. Unless it is slower than calling both std::isnan and std::isinf. And if it is, then they should really fix that!
(In reply to Darin Adler from comment #10) > (In reply to Darin Adler from comment #9) > > I prefer std::isfinite for checks like this. > > Unless it is slower than calling both std::isnan and std::isinf. And if it > is, then they should really fix that! Humm, I think it's a good idea to benchmark that, we can even compare the two approaches against using fpclassify.