LGTM with nits

https://codereview.chromium.org/191293013/diff/50001/src/a64/lithium-codegen-a64.cc
File src/a64/lithium-codegen-a64.cc (right):

https://codereview.chromium.org/191293013/diff/50001/src/a64/lithium-codegen-a64.cc#newcode2621
src/a64/lithium-codegen-a64.cc:2621: Abs(divisor) != 1) {
Hmmm, I consciously did not use Abs here (and similar places) because I
wanted to avoid Abs(kMinInt). This "works" currently, but not if we add
an ASSERT in Abs later.

I would prefer the previous version, but it's up to you...

https://codereview.chromium.org/191293013/diff/50001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/191293013/diff/50001/src/hydrogen-instructions.h#newcode858
src/hydrogen-instructions.h:858: bool RangeCanInclude(int value) const {
Can't we kill this, too?

https://codereview.chromium.org/191293013/

--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to