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;
On 2014/05/02 10:01:18, ulan wrote:
Simpler condition: !(val->IsBitwise() || val->IsAdd() || val->IsSub())
Done.
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);
On 2014/05/02 10:01:18, ulan wrote:
You can inline hinstr->CheckFlag(HValue::kCanOverflow) below without
can_overflow
Done.
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.cc#newcode2148
src/arm64/lithium-arm64.cc:2148: UNREACHABLE();
On 2014/05/02 10:01:18, ulan wrote:
nit: I'd ASSERT(hinstr->IsSub()) above.
Done.
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:
On 2014/05/02 10:01:18, ulan wrote:
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.
Done.
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);
On 2014/05/02 10:01:18, ulan wrote:
Did you mean && instead of ||?
'||' was intended. IsImmAddSub() works with unsigned values.
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-arm64.h#newcode3104
src/arm64/lithium-arm64.h:3104: UNREACHABLE();
On 2014/05/02 10:01:18, ulan wrote:
nit: I'd slightly prefer a more compact ASSERT(instr->IsBitwise())
above without
this branch.
Done.
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);
On 2014/05/02 10:01:18, ulan wrote:
Could you name the 0x1f constant since it is used in other places too?
Abstracted the constant and used a helper.
https://codereview.chromium.org/257203002/diff/1/src/arm64/lithium-codegen-arm64.cc#newcode1270
src/arm64/lithium-codegen-arm64.cc:1270: }
On 2014/05/02 10:01:18, ulan wrote:
Two empty lines after function.
Done.
https://codereview.chromium.org/257203002/
--
--
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.
