https://codereview.chromium.org/425003004/diff/20001/src/compiler/js-typed-lowering.cc
File src/compiler/js-typed-lowering.cc (right):
https://codereview.chromium.org/425003004/diff/20001/src/compiler/js-typed-lowering.cc#newcode518
src/compiler/js-typed-lowering.cc:518: if
(input_type->Is(Type::Number())) {
Why is this duplicated?
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc
File src/compiler/simplified-lowering.cc (left):
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#oldcode322
src/compiler/simplified-lowering.cc:322: case IrOpcode::kLoadField:
Why did these cases disappear?
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc
File src/compiler/simplified-lowering.cc (right):
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode32
src/compiler/simplified-lowering.cc:32: enum Phase {
I wish we could somehow factor this using some form of parameterisation,
and not a mode flag... :(
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode60
src/compiler/simplified-lowering.cc:60: RepTypeUnion output : 16; //
Output type of the node.
Why is this bitset larger than the other?
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode232
src/compiler/simplified-lowering.cc:232: // Helpers for specific types
of binops.
Nit: not sure these are worth it, but I leave it up to you.
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode247
src/compiler/simplified-lowering.cc:247: int values =
node->op()->InputCount();
Doesn't this include non-values?
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode252
src/compiler/simplified-lowering.cc:252: ProcessInput(node,
iter.index(), values > 0 ? use : 0);
Oh, this breaks the node properties abstraction quite nastily.
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode310
src/compiler/simplified-lowering.cc:310: return VisitLeaf(node, rBit);
Nit: could combine these two cases.
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode312
src/compiler/simplified-lowering.cc:312: return VisitLeaf(node, rBit);
Why rBit?
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode343
src/compiler/simplified-lowering.cc:343:
//------------------------------------------------------------------
Nit: weird indentation
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode369
src/compiler/simplified-lowering.cc:369: // BooleanNot(x: rTagged) =>
WordEqual(x, #false)
Wait, why is BooleanNot overloaded on JS Booleans?
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode446
src/compiler/simplified-lowering.cc:446: // Propage a type to the input,
but not a representation.
Typo: Propagate
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode552
src/compiler/simplified-lowering.cc:552: // We use unsigned int32 as the
output type for shift right, though
This comment is confusing. Unsigned seems adequate here. Is this a bogus
copy & paste from the next case, perhaps?
https://codereview.chromium.org/425003004/diff/20001/src/compiler/simplified-lowering.cc#newcode957
src/compiler/simplified-lowering.cc:957: void
SimplifiedLowering::Lower(Node* node) {}
What's this still good for?
https://codereview.chromium.org/425003004/
--
--
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.
