Adds logical operations to arm64.

Adds logical operations to arm64.

R=regis@google.com

Committed: https://code.google.com/p/dart/source/detail?r=34901

[zra, 2014-04-09 18:21:18]

Assembler::IsImmLogical and Instruction::ImmLogical are based on V8. I'm not
sure whether/how I should note that in a comment.

[regis, 2014-04-09 20:49:49]

LGTM

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.cc
File runtime/vm/assembler_arm64.cc (right):

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.c...
runtime/vm/assembler_arm64.cc:77: static int CountLeadingZeros(uint64_t value,
int width) {
Can width be 32 with a value that does not fit in 32 bits?
I not, I would add an assert.
A comment explaining what this function expects and returns would be good.

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.c...
runtime/vm/assembler_arm64.cc:86: }
There are faster ways of doing this.
A much simpler test + loop would work (modulo handling of width):

if (value == 0) return width;
int count = 0;
while (value >>= 1) count++;
return count;

You can even do this without a loop, but it may not be worth it.
Example:
https://github.com/gagern/gnulib/blob/master/lib/count-leading-zeros.h

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.h
File runtime/vm/assembler_arm64.h (right):

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.h...
runtime/vm/assembler_arm64.h:220: // Call Assembler::IsImmLogical to find values
for these arguments.
I do not understand this comment.

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.h...
runtime/vm/assembler_arm64.h:355: const bool immok = IsImmLogical(imm,
kXRegSizeInBits, &n, &imm_s, &imm_r);
You could also use a pattern similar to ShifterOperand::CanHold on arm:

Operand imm_op;
const bool immok = IsImmLogical(imm, kXRegSizeInBits, &imm_op);
ASSERT(immok);
EmitLogicalImmOp(ANDI, rd, rn, imm_op, kDoubleWord);

https://codereview.chromium.org/231373003/diff/1/runtime/vm/constants_arm64.h
File runtime/vm/constants_arm64.h (right):

https://codereview.chromium.org/231373003/diff/1/runtime/vm/constants_arm64.h...
runtime/vm/constants_arm64.h:629: // the following table:
What is the relation between the resulting integer and S and R in the table
below?

https://codereview.chromium.org/231373003/diff/1/runtime/vm/constants_arm64.h...
runtime/vm/constants_arm64.h:642: // 32 or 64-bit value, depending on
destination register width.
I see now.

[zra, 2014-04-09 22:14:27]

Thanks!

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.cc
File runtime/vm/assembler_arm64.cc (right):

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.c...
runtime/vm/assembler_arm64.cc:77: static int CountLeadingZeros(uint64_t value,
int width) {
On 2014/04/09 20:49:49, regis wrote:
> Can width be 32 with a value that does not fit in 32 bits?
> I not, I would add an assert.
> A comment explaining what this function expects and returns would be good.

Added an ASSERT in IsImmLogical.

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.c...
runtime/vm/assembler_arm64.cc:86: }
On 2014/04/09 20:49:49, regis wrote:
> There are faster ways of doing this.
> A much simpler test + loop would work (modulo handling of width):
> 
> if (value == 0) return width;
> int count = 0;
> while (value >>= 1) count++;
> return count;
> 
> You can even do this without a loop, but it may not be worth it.
> Example:
> https://github.com/gagern/gnulib/blob/master/lib/count-leading-zeros.h

Done.

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.h
File runtime/vm/assembler_arm64.h (right):

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.h...
runtime/vm/assembler_arm64.h:220: // Call Assembler::IsImmLogical to find values
for these arguments.
On 2014/04/09 20:49:49, regis wrote:
> I do not understand this comment.

Tried to clarify.

https://codereview.chromium.org/231373003/diff/1/runtime/vm/assembler_arm64.h...
runtime/vm/assembler_arm64.h:355: const bool immok = IsImmLogical(imm,
kXRegSizeInBits, &n, &imm_s, &imm_r);
On 2014/04/09 20:49:49, regis wrote:
> You could also use a pattern similar to ShifterOperand::CanHold on arm:
> 
> Operand imm_op;
> const bool immok = IsImmLogical(imm, kXRegSizeInBits, &imm_op);
> ASSERT(immok);
> EmitLogicalImmOp(ANDI, rd, rn, imm_op, kDoubleWord);

Done.

[zra, 2014-04-09 22:15:09]

Committed patchset #3 manually as r34901 (presubmit successful).