Subzero: Improve register availability peephole for function return values.

Subzero: Improve register availability peephole for function return values.

Originally, a call instruction was lowered like this:

  // %result = call @foo(...)
  %t1:eax = call foo
  %result = %t1:eax

Because t1 is pre-colored, it is not available as a substitution if the following instruction uses %result as a source operand.

To improve this, we copy it through an intermediate temporary:

  // %result = call @foo(...)
  %t1:eax = call foo
  %t2 = %t1:eax
  %result = %t2

BUG= none
R=eholk@chromium.org

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

[Jim Stichnoth, 2016-06-14 22:42:35]

Description was changed from

==========
Subzero: Improve register availability peephole for function return values.

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

to

==========
Subzero: Improve register availability peephole for function return values.

Originally, a call instruction was lowered like this:

  // %result = call @foo(...)
  %t1:eax = call foo
  %result = %t1:eax

Because t1 is pre-colored, it is not available as a substitution if the
following instruction uses %result as a source operand.

To improve this, we copy it through an intermediate temporary:

  // %result = call @foo(...)
  %t1:eax = call foo
  %t2 = %t1:eax
  %result = %t2

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

[Jim Stichnoth, 2016-06-14 22:44:03]

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

[Eric Holk, 2016-06-15 00:14:38]

Out of curiosity, did you measure whether this change has any impact on compile
time or generated code speed?

https://codereview.chromium.org/2064073005/diff/20001/src/IceTargetLoweringX8...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/2064073005/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:2733: Variable *TmpHi = nullptr;
Why is it okay for TmpHi to be NULL?

[Jim Stichnoth, 2016-06-15 00:35:31]

https://codereview.chromium.org/2064073005/diff/20001/src/IceTargetLoweringX8...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/2064073005/diff/20001/src/IceTargetLoweringX8...
src/IceTargetLoweringX86BaseImpl.h:2733: Variable *TmpHi = nullptr;
On 2016/06/15 00:14:38, Eric Holk wrote:
> Why is it okay for TmpHi to be NULL?

The _mov() method is "special" in that if the dest arg is nullptr it
automatically creates a new temporary of the appropriate type via makeReg(). 
The same is being done for "Variable *Tmp = nullptr;" above.

[Jim Stichnoth, 2016-06-15 00:40:56]

On 2016/06/15 00:14:38, Eric Holk wrote:
> Out of curiosity, did you measure whether this change has any impact on
compile
> time or generated code speed?

Right, I forgot to mention that.

My measurements showed about 0.75% spec2k geomean improvement.  I didn't do any
compile-time measurements.  Frankly, I'm surprised at the gain since this
pattern didn't really show up in the hot basic blocks I looked at, but maybe
there was some cumulative effect in the long tail.

[Eric Holk, 2016-06-15 00:47:43]

lgtm

[Jim Stichnoth, 2016-06-15 05:21:41]

Description was changed from

==========
Subzero: Improve register availability peephole for function return values.

Originally, a call instruction was lowered like this:

  // %result = call @foo(...)
  %t1:eax = call foo
  %result = %t1:eax

Because t1 is pre-colored, it is not available as a substitution if the
following instruction uses %result as a source operand.

To improve this, we copy it through an intermediate temporary:

  // %result = call @foo(...)
  %t1:eax = call foo
  %t2 = %t1:eax
  %result = %t2

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

to

==========
Subzero: Improve register availability peephole for function return values.

Originally, a call instruction was lowered like this:

  // %result = call @foo(...)
  %t1:eax = call foo
  %result = %t1:eax

Because t1 is pre-colored, it is not available as a substitution if the
following instruction uses %result as a source operand.

To improve this, we copy it through an intermediate temporary:

  // %result = call @foo(...)
  %t1:eax = call foo
  %t2 = %t1:eax
  %result = %t2

BUG= none
R=eholk@chromium.org

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

[Jim Stichnoth, 2016-06-15 05:21:43]

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