[turbofan] Create ExplicitOperands to specify operands without virtual registers

[turbofan] Create ExplicitOperands to specify operands without virtual registers

Up until now, if one wanted to specify an explicit stack location                                                                                                                                                                                                or register as an operand for an instruction, it had to also be
explicitly associated with a virtual register as a so-called
FixedRegister or FixedStackSlot.

For the implementation of tail calls, the plan is to use the gap
resolver needs to shuffle stack locations from the caller to the
tail-called callee. In order to do this, it must be possible to
explicitly address operand locations on the stack that are not
associated with virtual registers.

This CL introduces ExplictOperands, which can specify a specific
register or stack location that is not associated with virtual
register. This will allow tail calls to specify the target
locations for the necessary stack moves in the gap for the tail
call without the core register allocation having to know about
the target of the stack moves at all.

In the process this CL:
* creates a new Operand kind, ExplicitOperand, with which
  instructions can specify register and stack slots without an
  associated virtual register.
* creates a LocationOperand class from which AllocatedOperand and
  ExplicitOperand are derived and provides a common interface to
  get Register, DoubleRegister and spill slot information.
* removes RegisterOperand, DoubleRegisterOperand,
  StackSlotOperand and DoubleStackSlotOperand, they are subsumed
  by LocationOperand.
* addresses a cleanup TODO in AllocatedOperand to reduce the
  redundancy of AllocatedOperand::Kind by using machine_type() to
  determine if an operand corresponds to a general purpose or
  double register.

BUG=v8:4076
LOG=n

Committed: https://crrev.com/f1aa556278cf833960bd8f9ed96d4168f94478c3
Cr-Commit-Position: refs/heads/master@{#31603}

[danno, 2015-10-15 14:27:34]

danno@chromium.org changed reviewers:
+ bmeurer@chromium.org, jarin@chromium.org

[danno, 2015-10-15 14:27:44]

PTAL

[Mircea Trofin, 2015-10-15 15:05:38]

mtrofin@chromium.org changed reviewers:
+ mtrofin@chromium.org

[Mircea Trofin, 2015-10-15 15:05:38]

https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instructio...
File src/compiler/instruction.cc (right):

https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instructio...
src/compiler/instruction.cc:108: case InstructionOperand::EXPLICIT:
Would it help debugging if Explicit had some extra indication when printing out,
maybe something like "v1(E, <etc.>)"

https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instruction.h
File src/compiler/instruction.h (right):

https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instructio...
src/compiler/instruction.h:46: INSTRUCTION_OPERAND_PREDICATE(Explicit, EXPLICIT)
Maybe this would be a good time to add a short description here as to what each
operand means, it should improve code comprehension.

https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
File test/unittests/compiler/move-optimizer-unittest.cc (right):

https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
test/unittests/compiler/move-optimizer-unittest.cc:75: op.value_);
Should we also have a unit test where the move optimizer encounters an
ExplicitOperand?

https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
File test/unittests/compiler/register-allocator-unittest.cc (right):

https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
test/unittests/compiler/register-allocator-unittest.cc:47: test_op.value_ ==
InstructionSequenceTest::kNoValue);
Should we also have a unit test where the register allocator encounters an
ExplicitOperand?

[Jarin, 2015-10-16 09:25:06]

The code looks good, but it really needs to have test coverage. At least the gap
resolver should be covered (which is the intended use), but I would also like to
have tests for the bits that Mircea suggested.

[Benedikt Meurer, 2015-10-19 04:28:09]

bmeurer@chromium.org changed reviewers:
- bmeurer@chromium.org

[Benedikt Meurer, 2015-10-19 04:28:09]

bmeurer@chromium.org changed required reviewers:
+ jarin@chromium.org, mtrofin@chromium.org

[danno, 2015-10-23 08:07:50]

Please take another look. Feedback addressed. Test coverage added for move
optimizer and gap-resolver, none in the register allocator itself because
ExplicitOperands should never leak into there.

https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instructio...
File src/compiler/instruction.cc (right):

https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instructio...
src/compiler/instruction.cc:108: case InstructionOperand::EXPLICIT:
On 2015/10/15 at 15:05:38, Mircea Trofin wrote:
> Would it help debugging if Explicit had some extra indication when printing
out, maybe something like "v1(E, <etc.>)"

Done

https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instruction.h
File src/compiler/instruction.h (right):

https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instructio...
src/compiler/instruction.h:46: INSTRUCTION_OPERAND_PREDICATE(Explicit, EXPLICIT)
On 2015/10/15 at 15:05:38, Mircea Trofin wrote:
> Maybe this would be a good time to add a short description here as to what
each operand means, it should improve code comprehension.

Done

https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
File test/unittests/compiler/move-optimizer-unittest.cc (right):

https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
test/unittests/compiler/move-optimizer-unittest.cc:75: op.value_);
On 2015/10/15 at 15:05:38, Mircea Trofin wrote:
> Should we also have a unit test where the move optimizer encounters an
ExplicitOperand?

Done

https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
File test/unittests/compiler/register-allocator-unittest.cc (right):

https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
test/unittests/compiler/register-allocator-unittest.cc:47: test_op.value_ ==
InstructionSequenceTest::kNoValue);
On 2015/10/15 at 15:05:38, Mircea Trofin wrote:
> Should we also have a unit test where the register allocator encounters an
ExplicitOperand?

The register allocator itself should never see ExplicitOperands, if it does it
will DCHECK fail when converting the ExplicitOperand to an AllocatedOperand.
I've changed this test to not use LocationOperand so that it's clear that only
AllocatedOperands are involved.

[danno, 2015-10-26 13:35:04]

On 2015/10/23 at 08:07:50, danno wrote:
> Please take another look. Feedback addressed. Test coverage added for move
optimizer and gap-resolver, none in the register allocator itself because
ExplicitOperands should never leak into there.
> 
>
https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instructio...
> File src/compiler/instruction.cc (right):
> 
>
https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instructio...
> src/compiler/instruction.cc:108: case InstructionOperand::EXPLICIT:
> On 2015/10/15 at 15:05:38, Mircea Trofin wrote:
> > Would it help debugging if Explicit had some extra indication when printing
out, maybe something like "v1(E, <etc.>)"
> 
> Done
> 
>
https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instruction.h
> File src/compiler/instruction.h (right):
> 
>
https://codereview.chromium.org/1389373002/diff/80001/src/compiler/instructio...
> src/compiler/instruction.h:46: INSTRUCTION_OPERAND_PREDICATE(Explicit,
EXPLICIT)
> On 2015/10/15 at 15:05:38, Mircea Trofin wrote:
> > Maybe this would be a good time to add a short description here as to what
each operand means, it should improve code comprehension.
> 
> Done
> 
>
https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
> File test/unittests/compiler/move-optimizer-unittest.cc (right):
> 
>
https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
> test/unittests/compiler/move-optimizer-unittest.cc:75: op.value_);
> On 2015/10/15 at 15:05:38, Mircea Trofin wrote:
> > Should we also have a unit test where the move optimizer encounters an
ExplicitOperand?
> 
> Done
> 
>
https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
> File test/unittests/compiler/register-allocator-unittest.cc (right):
> 
>
https://codereview.chromium.org/1389373002/diff/80001/test/unittests/compiler...
> test/unittests/compiler/register-allocator-unittest.cc:47: test_op.value_ ==
InstructionSequenceTest::kNoValue);
> On 2015/10/15 at 15:05:38, Mircea Trofin wrote:
> > Should we also have a unit test where the register allocator encounters an
ExplicitOperand?
> 
> The register allocator itself should never see ExplicitOperands, if it does it
will DCHECK fail when converting the ExplicitOperand to an AllocatedOperand.
I've changed this test to not use LocationOperand so that it's clear that only
AllocatedOperands are involved.

Friendly ping...

[Jarin, 2015-10-26 13:37:02]

lgtm with a question.

https://codereview.chromium.org/1389373002/diff/140001/src/compiler/instructi...
File src/compiler/instruction-selector-impl.h (right):

https://codereview.chromium.org/1389373002/diff/140001/src/compiler/instructi...
src/compiler/instruction-selector-impl.h:125: return
ExplicitOperand(LocationOperand::REGISTER, machine_type, reg.code());
Maybe I missed it somewhere, but should not we only allow explicit operands for
non-allocatable registers?

https://codereview.chromium.org/1389373002/diff/140001/src/compiler/instructi...
File src/compiler/instruction.h (right):

https://codereview.chromium.org/1389373002/diff/140001/src/compiler/instructi...
src/compiler/instruction.h:61: // register allocator and are always associated
with a virtual register.
Thank you, the descriptions are great!

[Mircea Trofin, 2015-10-26 21:35:07]

On 2015/10/26 13:37:02, Jarin wrote:
> lgtm with a question.
> 
>
https://codereview.chromium.org/1389373002/diff/140001/src/compiler/instructi...
> File src/compiler/instruction-selector-impl.h (right):
> 
>
https://codereview.chromium.org/1389373002/diff/140001/src/compiler/instructi...
> src/compiler/instruction-selector-impl.h:125: return
> ExplicitOperand(LocationOperand::REGISTER, machine_type, reg.code());
> Maybe I missed it somewhere, but should not we only allow explicit operands
for
> non-allocatable registers?
> 
>
https://codereview.chromium.org/1389373002/diff/140001/src/compiler/instructi...
> File src/compiler/instruction.h (right):
> 
>
https://codereview.chromium.org/1389373002/diff/140001/src/compiler/instructi...
> src/compiler/instruction.h:61: // register allocator and are always associated
> with a virtual register.
> Thank you, the descriptions are great!

lgtm!

[danno, 2015-10-27 13:15:28]

The CQ bit was checked by danno@chromium.org

[danno, 2015-10-27 13:15:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
mtrofin@chromium.org, jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/1389373002/#ps160001
(title: "Review feedback")

[commit-bot: I haz the power, 2015-10-27 13:15:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1389373002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1389373002/160001

[commit-bot: I haz the power, 2015-10-27 13:26:39]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-10-27 13:27:06]

Patchset 9 (id:??) landed as
https://crrev.com/f1aa556278cf833960bd8f9ed96d4168f94478c3
Cr-Commit-Position: refs/heads/master@{#31603}