Fix result of LCodeGen::DoWrapReceiver for strict functions and builtins.

Fix result of LCodeGen::DoWrapReceiver for strict functions and builtins.

BUG=362128
LOG=Y
TEST=mjsunit/regress/regress-362128
R=jacob.bramley@arm.com

Committed: https://code.google.com/p/v8/source/detail?r=20723

[ulan, 2014-04-11 17:32:01]

PTAL

[jbramley, 2014-04-14 08:59:52]

It looks good; I just had one suggestion.

I think it's possible to avoid copy_receiver (and thus the branch over it) by
speculatively copying the receiver before the Tbnz instructions, but this
doesn't make the code any smaller, and it might not be faster either.

https://codereview.chromium.org/226363007/diff/1/src/arm64/lithium-codegen-ar...
File src/arm64/lithium-codegen-arm64.cc (right):

https://codereview.chromium.org/226363007/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:5849: __ Mov(result, receiver);
You can skip this move now, and do __ B(ge, &copy_receiver).

[ulan, 2014-04-14 11:12:53]

> I think it's possible to avoid copy_receiver (and thus the branch over it) by
speculatively copying the receiver before the Tbnz instructions
Would this require using an additional register? Since Tbnz instructions use the
"result" register.

https://codereview.chromium.org/226363007/diff/1/src/arm64/lithium-codegen-ar...
File src/arm64/lithium-codegen-arm64.cc (right):

https://codereview.chromium.org/226363007/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:5849: __ Mov(result, receiver);
On 2014/04/14 08:59:53, jbramley wrote:
> You can skip this move now, and do __ B(ge, &copy_receiver).

Done.

[jbramley, 2014-04-14 11:21:12]

On 2014/04/14 11:12:53, ulan wrote:
> > I think it's possible to avoid copy_receiver (and thus the branch over it)
by
> speculatively copying the receiver before the Tbnz instructions
> Would this require using an additional register? Since Tbnz instructions use
the
> "result" register.

No, I meant like this:

__ Tst(result, (1 << SharedFunctionInfo::kStrictModeFunction) |
               (1 << SharedFunctionInfo::kNative));
__ Mov(result, receiver);    // Speculative.
__ B(ne, &done);

I haven't thought through all the details though. The tst actually generates two
instructions because I don't think that immediate fits in a logical immediate
operand.

[ulan, 2014-04-14 11:49:28]

On 2014/04/14 11:21:12, jbramley wrote:
> On 2014/04/14 11:12:53, ulan wrote:
> > > I think it's possible to avoid copy_receiver (and thus the branch over it)
> by
> > speculatively copying the receiver before the Tbnz instructions
> > Would this require using an additional register? Since Tbnz instructions use
> the
> > "result" register.
> 
> No, I meant like this:
> 
> __ Tst(result, (1 << SharedFunctionInfo::kStrictModeFunction) |
>                (1 << SharedFunctionInfo::kNative));
> __ Mov(result, receiver);    // Speculative.
> __ B(ne, &done);
> 
> I haven't thought through all the details though. The tst actually generates
two
> instructions because I don't think that immediate fits in a logical immediate
> operand.

You're right, it uses two instructions:
0x5fb791f8   152  movz x16, #0x90
0x5fb791fc   156  tst x4, x16

Should I land the current version?

[jbramley, 2014-04-14 11:51:10]

On 2014/04/14 11:49:28, ulan wrote:
> On 2014/04/14 11:21:12, jbramley wrote:
> > On 2014/04/14 11:12:53, ulan wrote:
> > > > I think it's possible to avoid copy_receiver (and thus the branch over
it)
> > by
> > > speculatively copying the receiver before the Tbnz instructions
> > > Would this require using an additional register? Since Tbnz instructions
use
> > the
> > > "result" register.
> > 
> > No, I meant like this:
> > 
> > __ Tst(result, (1 << SharedFunctionInfo::kStrictModeFunction) |
> >                (1 << SharedFunctionInfo::kNative));
> > __ Mov(result, receiver);    // Speculative.
> > __ B(ne, &done);
> > 
> > I haven't thought through all the details though. The tst actually generates
> two
> > instructions because I don't think that immediate fits in a logical
immediate
> > operand.
> 
> You're right, it uses two instructions:
> 0x5fb791f8   152  movz x16, #0x90
> 0x5fb791fc   156  tst x4, x16
> 
> Should I land the current version?

Yep, I think so. I can't give an lgtm that will help you though.

[ulan, 2014-04-14 11:58:31]

Committed patchset #2 manually as r20723 (presubmit successful).

[ulan, 2014-04-14 12:01:45]

On 2014/04/14 11:58:31, ulan wrote:
> Committed patchset #2 manually as r20723 (presubmit successful).

> Yep, I think so. I can't give an lgtm that will help you though.
This is strange, but I was able to land. :) Looks like the presubmit is not
checking for OWNERS in arm64. I'll check with machenbach@

[ulan, 2014-04-14 12:06:01]

On 2014/04/14 12:01:45, ulan wrote:
> On 2014/04/14 11:58:31, ulan wrote:
> > Committed patchset #2 manually as r20723 (presubmit successful).
> 
> > Yep, I think so. I can't give an lgtm that will help you though.
> This is strange, but I was able to land. :) Looks like the presubmit is not
> checking for OWNERS in arm64. I'll check with machenbach@

Michael clarified that (committer OR reviewer) should be in OWNERS, not
necessarily both.