Subzero: Merge SpillVariable functionality directly into Variable.

Subzero: Merge SpillVariable functionality directly into Variable.

Specifically, this is the ability to link variable B to variable A, such that if neither A nor B get a register assignment, they share a stack slot.

This CL just refactors and keeps basically the same functionality, in preparation for new work on variable splitting.

BUG= none
R=jpp@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=e343e0660aac84a8c6369f67fde19b401ca00104

[Jim Stichnoth, 2016-06-29 02:25:44]

Description was changed from

==========
Subzero: Merge SpillVariable functionality directly into Variable.

BUG= none
==========

to

==========
Subzero: Merge SpillVariable functionality directly into Variable.

Specifically, this is the ability to link variable B to variable A, such that if
neither A nor B get a register assignment, they share a stack slot.

This CL just refactors and keeps basically the same functionality, in
preparation for new work on variable splitting.

BUG= none
==========

[Jim Stichnoth, 2016-06-29 02:28:45]

stichnot@chromium.org changed reviewers:
+ eholk@chromium.org, jpp@chromium.org, kschimpf@google.com, manasijm@google.com

[John, 2016-06-29 03:50:44]

lgtm

everything is optional...

https://codereview.chromium.org/2107073002/diff/1/src/IceOperand.h
File src/IceOperand.h (right):

https://codereview.chromium.org/2107073002/diff/1/src/IceOperand.h#newcode781
src/IceOperand.h:781: Var = Var->getLinkedTo();
I understand why you're using the getter here, but this "hides" the fact (that
is expressed by the comment above) that:

if Var.LinkedTo != nullptr --> Var.LinkedTo.LinkedTo == nullptr

I would probably just do:
Var = Var.LinkedTo

(and maybe put an assert before it.)

Up to you

https://codereview.chromium.org/2107073002/diff/1/src/IceOperand.h#newcode787
src/IceOperand.h:787: assert(LinkedTo->getLinkedTo() == nullptr);
perhaps:

assert(LinkedTo.LinkedTo == nullptr)

?

https://codereview.chromium.org/2107073002/diff/1/src/IceTargetLoweringX86Bas...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/2107073002/diff/1/src/IceTargetLoweringX86Bas...
src/IceTargetLoweringX86BaseImpl.h:1047: // TODO(stichnot): This assert won't
necessarily be true in the
why?

[Jim Stichnoth, 2016-06-29 04:29:35]

https://codereview.chromium.org/2107073002/diff/1/src/IceOperand.h
File src/IceOperand.h (right):

https://codereview.chromium.org/2107073002/diff/1/src/IceOperand.h#newcode781
src/IceOperand.h:781: Var = Var->getLinkedTo();
On 2016/06/29 03:50:44, John wrote:
> I understand why you're using the getter here, but this "hides" the fact (that
> is expressed by the comment above) that:
> 
> if Var.LinkedTo != nullptr --> Var.LinkedTo.LinkedTo == nullptr
> 
> I would probably just do:
> Var = Var.LinkedTo
> 
> (and maybe put an assert before it.)
> 
> Up to you

Done.

https://codereview.chromium.org/2107073002/diff/1/src/IceOperand.h#newcode787
src/IceOperand.h:787: assert(LinkedTo->getLinkedTo() == nullptr);
On 2016/06/29 03:50:43, John wrote:
> perhaps:
> 
> assert(LinkedTo.LinkedTo == nullptr)
> 
> ?

Done.

https://codereview.chromium.org/2107073002/diff/1/src/IceTargetLoweringX86Bas...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/2107073002/diff/1/src/IceTargetLoweringX86Bas...
src/IceTargetLoweringX86BaseImpl.h:1047: // TODO(stichnot): This assert won't
necessarily be true in the
On 2016/06/29 03:50:44, John wrote:
> why?

The next CL will aim to do local live range splitting within a basic block to
help improve register allocation - specifically when a variable is referenced
multiple times in a block but didn't get a register because of its large live
range.  It will do this by making multiple copies of the variable and hope that
at least some will get registers.  If they don't, this LinkedTo mechanism keeps
the copies at the same stack location so we don't end up with extra stack
shuffles.

With that use, the variable won't be forbidden from having a register.

[Jim Stichnoth, 2016-06-29 04:40:38]

Description was changed from

==========
Subzero: Merge SpillVariable functionality directly into Variable.

Specifically, this is the ability to link variable B to variable A, such that if
neither A nor B get a register assignment, they share a stack slot.

This CL just refactors and keeps basically the same functionality, in
preparation for new work on variable splitting.

BUG= none
==========

to

==========
Subzero: Merge SpillVariable functionality directly into Variable.

Specifically, this is the ability to link variable B to variable A, such that if
neither A nor B get a register assignment, they share a stack slot.

This CL just refactors and keeps basically the same functionality, in
preparation for new work on variable splitting.

BUG= none
R=jpp@chromium.org

Committed:
https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a...
==========

[Jim Stichnoth, 2016-06-29 04:40:39]

Committed patchset #2 (id:20001) manually as
e343e0660aac84a8c6369f67fde19b401ca00104 (presubmit successful).