Looking good, just a couple of comments about the simplified lowering.
https://codereview.chromium.org/612043003/diff/270001/src/compiler/simplified-lowering.cc
File src/compiler/simplified-lowering.cc (right):
https://codereview.chromium.org/612043003/diff/270001/src/compiler/simplified-lowering.cc#newcode364
src/compiler/simplified-lowering.cc:364: Node* is_smi =
jsgraph_->graph()->NewNode(
For posterity: There might be a more optimal lowering if the output
representation is determined to be kRepBit, but lets land it like this
for now.
https://codereview.chromium.org/612043003/diff/270001/src/compiler/simplified-lowering.cc#newcode382
src/compiler/simplified-lowering.cc:382: jsgraph_->machine()->Is32()
nit: There already should be IntLessThanOrEqual() shortcut for this.
https://codereview.chromium.org/612043003/diff/270001/src/compiler/simplified-lowering.cc#newcode872
src/compiler/simplified-lowering.cc:872: return VisitIsSmi(node);
Please move this into the section for simplified operators, we are in
the section for machine operators here.
Also to be consistent with the rest of the code, the processing of
inputs and setting of output type (i.e. the "VisitFoo" method family)
should be done inline, whereas the actual lowering could be done in a
helper (i.e. the "DoFoo" method familiy).
https://codereview.chromium.org/612043003/
--
--
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.
