[turbofan] Allow TempReg to be SameAsFirst and FromVreg

[turbofan] Allow TempReg to be SameAsFirst and FromVreg

BUG=

Review-Url: https://codereview.chromium.org/2650813003
Cr-Commit-Position: refs/heads/master@{#42661}
Committed: https://chromium.googlesource.com/v8/v8/+/dde145054f846201e050ab60dc37c9d28a9e8ffc

[john.yan, 2017-01-23 20:03:33]

Description was changed from

==========
[turbofan] Allow TempReg to be SameAsFirst and FromVreg

BUG=
==========

to

==========
[turbofan] Allow TempReg to be SameAsFirst and FromVreg

BUG=
==========

[john.yan, 2017-01-23 20:03:33]

jyan@ca.ibm.com changed reviewers:
+ bmeurer@chromium.org, mtrofin@chromium.org

[john.yan, 2017-01-23 20:03:43]

The CQ bit was checked by jyan@ca.ibm.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 20:03:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-23 20:40:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-23 20:40:20]

Dry run: This issue passed the CQ dry run.

[john.yan, 2017-01-24 02:48:59]

ptal

[Mircea Trofin, 2017-01-24 03:16:52]

On 2017/01/24 02:48:59, john.yan wrote:
> ptal

My preference would be for this (or any) change to come paired with code
exercising it, plus some form of tests. 

For instance, you could couple it together with your other change (the one that
sparked this), and add a few tests ensuring you got the expected code gen.

I can appreciate, though, the tension with a desire for small, incremental
changes - I'll let Benedikt weigh in.

[Benedikt Meurer, 2017-01-24 06:47:59]

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

[Jarin, 2017-01-24 08:14:04]

https://codereview.chromium.org/2650813003/diff/1/src/compiler/instruction-se...
File src/compiler/instruction-selector-impl.h (right):

https://codereview.chromium.org/2650813003/diff/1/src/compiler/instruction-se...
src/compiler/instruction-selector-impl.h:197:
UnallocatedOperand::cast(reg).virtual_register());
I am not a big fan of taking the vreg from one operand and smashing it
elsewhere. How about exposing sequence()->NextVirtualRegister() and then have a
method that can create the operands from supplied virtual register?

I am imagining something like:

int AllocateVirtualRegister();
InstructionOperand DefineSameAsFirstForVreg(int vreg);
InstructionOperand UseRegisterForVreg(int vreg);

[john.yan, 2017-01-24 15:07:30]

https://codereview.chromium.org/2650813003/diff/1/src/compiler/instruction-se...
File src/compiler/instruction-selector-impl.h (right):

https://codereview.chromium.org/2650813003/diff/1/src/compiler/instruction-se...
src/compiler/instruction-selector-impl.h:197:
UnallocatedOperand::cast(reg).virtual_register());
On 2017/01/24 08:14:04, Jarin wrote:
> I am not a big fan of taking the vreg from one operand and smashing it
> elsewhere. How about exposing sequence()->NextVirtualRegister() and then have
a
> method that can create the operands from supplied virtual register?
> 
> I am imagining something like:
> 
> int AllocateVirtualRegister();
> InstructionOperand DefineSameAsFirstForVreg(int vreg);
> InstructionOperand UseRegisterForVreg(int vreg);

That sounds better to me. Thanks.

[john.yan, 2017-01-24 15:33:52]

On 2017/01/24 08:14:04, Jarin wrote:
>
https://codereview.chromium.org/2650813003/diff/1/src/compiler/instruction-se...
> File src/compiler/instruction-selector-impl.h (right):
> 
>
https://codereview.chromium.org/2650813003/diff/1/src/compiler/instruction-se...
> src/compiler/instruction-selector-impl.h:197:
> UnallocatedOperand::cast(reg).virtual_register());
> I am not a big fan of taking the vreg from one operand and smashing it
> elsewhere. How about exposing sequence()->NextVirtualRegister() and then have
a
> method that can create the operands from supplied virtual register?
> 
> I am imagining something like:
> 
> int AllocateVirtualRegister();
> InstructionOperand DefineSameAsFirstForVreg(int vreg);
> InstructionOperand UseRegisterForVreg(int vreg);

updated.

[Jarin, 2017-01-25 06:37:13]

lgtm

[john.yan, 2017-01-25 15:38:17]

The CQ bit was checked by jyan@ca.ibm.com

[commit-bot: I haz the power, 2017-01-25 15:38:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-25 16:11:10]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485358697626520,
"parent_rev": "c18d4216a431d54813e79a7719e17a30c8a3ec24", "commit_rev":
"dde145054f846201e050ab60dc37c9d28a9e8ffc"}

[commit-bot: I haz the power, 2017-01-25 16:11:15]

Description was changed from

==========
[turbofan] Allow TempReg to be SameAsFirst and FromVreg

BUG=
==========

to

==========
[turbofan] Allow TempReg to be SameAsFirst and FromVreg

BUG=

Review-Url: https://codereview.chromium.org/2650813003
Cr-Commit-Position: refs/heads/master@{#42661}
Committed:
https://chromium.googlesource.com/v8/v8/+/dde145054f846201e050ab60dc37c9d28a9...
==========

[commit-bot: I haz the power, 2017-01-25 16:11:17]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/dde145054f846201e050ab60dc37c9d28a9...