Although it adds a lot of complexity, I think 15% performance boost can
justify
landing it.
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.cc
File src/arm64/lithium-arm64.cc (right):
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.cc#newcode2047
src/arm64/lithium-arm64.cc:2047: if (!val->IsBitwise() && !(val->IsAdd()
|| val->IsSub())) return NULL;
Simpler condition: !(val->IsBitwise() || val->IsAdd() || val->IsSub())
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.cc#newcode2139
src/arm64/lithium-arm64.cc:2139: bool can_overflow =
hinstr->CheckFlag(HValue::kCanOverflow);
You can inline hinstr->CheckFlag(HValue::kCanOverflow) below without
can_overflow
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.cc#newcode2148
src/arm64/lithium-arm64.cc:2148: UNREACHABLE();
nit: I'd ASSERT(hinstr->IsSub()) above.
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.h
File src/arm64/lithium-arm64.h (right):
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.h#newcode563
src/arm64/lithium-arm64.h:563: public:
Google style guide doesn't allow non-pure multiple inheritance.
One possible solution is to turn each function that accepts
LShiftedRightOpInterface* into a template with L-instructions as a
template parameter.
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.h#newcode3098
src/arm64/lithium-arm64.h:3098: return Assembler::IsImmAddSub(imm) ||
Assembler::IsImmAddSub(-imm);
Did you mean && instead of ||?
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.h#newcode3104
src/arm64/lithium-arm64.h:3104: UNREACHABLE();
nit: I'd slightly prefer a more compact ASSERT(instr->IsBitwise()) above
without this branch.
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-codegen-arm64.cc
File src/arm64/lithium-codegen-arm64.cc (right):
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-codegen-arm64.cc#newcode1268
src/arm64/lithium-codegen-arm64.cc:1268:
ToInteger32(LConstantOperand::cast(shift_info->shift_amount())) & 0x1f);
Could you name the 0x1f constant since it is used in other places too?
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-codegen-arm64.cc#newcode1270
src/arm64/lithium-codegen-arm64.cc:1270: }
Two empty lines after function.
https://codereview.chromium.org/257203002/
--
--
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.
