On 2015/04/15 13:02:41, Sven Panne wrote:
Uh, oh... Looking at LChunkBuilder::DoMul for ARM64 (only on that platform,
why
do we do things differently here???),

Don't know, don't care, Lithium is deprecated.

we already enter undefined-behavior-land
when trying to compute the absolute value of kMinInt. :-P I think we need to
fix
this, making sure that kMinInt, -kMaxInt etc. are handled correctly. Otherwise
I'm not sure that we don't hit this assertion, or if we're lucky, or if we
really never fail the DCHECK, or...

Well, we'll only use the result of the Abs() call if the original value was
*not* kMinInt. Also, it just so happens that with our current compilers,
computing -kMinInt returns kMinInt, without any kitten-killing side effects.
I've stared at this code quite a bit and I'm convinced that it's currently safe. (We can pull the Abs() computating behind the "if (!end_range_constant)" check
if it let you sleep better.)

I agree that ideally we'd fix all of the uses of Abs() all over the code base (there are many more), but that's a pretty big yak shave for unclear benefit.
I'd rather land this CL as-is as an incremental step towards more
safety/correctness.

https://codereview.chromium.org/1058533007/

--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to