Optimize the typeof operator.

Optimize the typeof operator.

typeof was implemented as a runtime function. Calling it in
optimized code with a non-constant input becomes burdensome.

BUG=

Committed: https://crrev.com/7798548a8f2ef14e8320cc42effa86f0eb8e4294
Cr-Commit-Position: refs/heads/master@{#28260}

[mvstanton, 2015-04-28 16:32:38]

mvstanton@chromium.org changed reviewers:
+ verwaest@chromium.org

[mvstanton, 2015-04-28 16:32:39]

Hi Toon,
Here is the CL we discussed, providing some performance gains. PTAL,
--Michael

ps - I cc'd MIPS folks in case they have comments.

[paul.l..., 2015-04-28 17:03:36]

paul.lind@imgtec.com changed reviewers:
+ paul.lind@imgtec.com

[paul.l..., 2015-04-28 17:03:37]

Michael - a couple build problems to address ... looks like the trybots show
them ...

https://codereview.chromium.org/1114563003/diff/20001/src/mips/lithium-codege...
File src/mips/lithium-codegen-mips.cc (right):

https://codereview.chromium.org/1114563003/diff/20001/src/mips/lithium-codege...
src/mips/lithium-codegen-mips.cc:5561: __ mov(v0,
Immediate(isolate()->factory()->number_string()));
s/b: __ li(v0, Operand(isolate()->factory()->number_string()));

By convention, mips uses li 'load immediate' rather than mov.

https://codereview.chromium.org/1114563003/diff/20001/src/mips64/lithium-code...
File src/mips64/lithium-codegen-mips64.cc (right):

https://codereview.chromium.org/1114563003/diff/20001/src/mips64/lithium-code...
src/mips64/lithium-codegen-mips64.cc:5617: void LCodeGen::DoTypeof(LTypeof*
instr) {
Delete this duplicated function.

https://codereview.chromium.org/1114563003/diff/20001/src/mips64/lithium-code...
src/mips64/lithium-codegen-mips64.cc:5631: __ mov(v0,
Immediate(isolate()->factory()->number_string()));
s/b:  __ li(v0, Operand(isolate()->factory()->number_string()));

[mvstanton, 2015-04-29 06:37:24]

Thanks for the comments Paul, uploaded fixes.

https://codereview.chromium.org/1114563003/diff/20001/src/mips/lithium-codege...
File src/mips/lithium-codegen-mips.cc (right):

https://codereview.chromium.org/1114563003/diff/20001/src/mips/lithium-codege...
src/mips/lithium-codegen-mips.cc:5561: __ mov(v0,
Immediate(isolate()->factory()->number_string()));
On 2015/04/28 17:03:36, paul.l... wrote:
> s/b: __ li(v0, Operand(isolate()->factory()->number_string()));
> 
> By convention, mips uses li 'load immediate' rather than mov.

Done.

https://codereview.chromium.org/1114563003/diff/20001/src/mips64/lithium-code...
File src/mips64/lithium-codegen-mips64.cc (right):

https://codereview.chromium.org/1114563003/diff/20001/src/mips64/lithium-code...
src/mips64/lithium-codegen-mips64.cc:5617: void LCodeGen::DoTypeof(LTypeof*
instr) {
On 2015/04/28 17:03:37, paul.l... wrote:
> Delete this duplicated function.

Oh brother! :p Thanks.

https://codereview.chromium.org/1114563003/diff/20001/src/mips64/lithium-code...
src/mips64/lithium-codegen-mips64.cc:5631: __ mov(v0,
Immediate(isolate()->factory()->number_string()));
On 2015/04/28 17:03:37, paul.l... wrote:
> s/b:  __ li(v0, Operand(isolate()->factory()->number_string()));

Done.

[Michael Starzinger, 2015-04-29 08:15:34]

TurboFan changes look good.

https://codereview.chromium.org/1114563003/diff/40001/src/compiler/js-generic...
File src/compiler/js-generic-lowering.cc (right):

https://codereview.chromium.org/1114563003/diff/40001/src/compiler/js-generic...
src/compiler/js-generic-lowering.cc:278: ReplaceWithStubCall(node, callable,
CallDescriptor::kPatchableCallSite);
As discussed offline: If the Typeof-Stub doesn't patch the call-site (the way
ICs do), we can mark this as CallDescriptor::kNoFlags instead.

[Toon Verwaest, 2015-04-30 14:01:09]

https://codereview.chromium.org/1114563003/diff/40001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/1114563003/diff/40001/src/code-stubs-hydrogen...
src/code-stubs-hydrogen.cc:320: is_number.If<HCompareObjectEqAndBranch>(map,
number_map);
Any reason why you didn't use HCompareMap(object,
isolate()->factory()->heap_number_map())?

https://codereview.chromium.org/1114563003/diff/40001/src/code-stubs-hydrogen...
src/code-stubs-hydrogen.cc:325: // Is it an undetectable object?
This is only possible for JSObjects, so should probably be checked after all the
other checks?

https://codereview.chromium.org/1114563003/diff/40001/src/code-stubs-hydrogen...
src/code-stubs-hydrogen.cc:334: is_undetectable_test, graph()->GetConstant0(),
Token::NE);
HIsUndetectableAndBranch

https://codereview.chromium.org/1114563003/diff/40001/src/code-stubs-hydrogen...
src/code-stubs-hydrogen.cc:346: instance_type,
Add<HConstant>(FIRST_NONSTRING_TYPE), Token::LT);
<HCheckInstanceType>(string, HCheckInstanceType::IS_STRING)

[mvstanton, 2015-05-06 09:55:27]

Thanks Toon, here are comments addressed..

https://codereview.chromium.org/1114563003/diff/40001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/1114563003/diff/40001/src/code-stubs-hydrogen...
src/code-stubs-hydrogen.cc:320: is_number.If<HCompareObjectEqAndBranch>(map,
number_map);
On 2015/04/30 14:01:09, Toon Verwaest wrote:
> Any reason why you didn't use HCompareMap(object,
> isolate()->factory()->heap_number_map())?

Done.

https://codereview.chromium.org/1114563003/diff/40001/src/code-stubs-hydrogen...
src/code-stubs-hydrogen.cc:325: // Is it an undetectable object?
On 2015/04/30 14:01:08, Toon Verwaest wrote:
> This is only possible for JSObjects, so should probably be checked after all
the
> other checks?

Done.

https://codereview.chromium.org/1114563003/diff/40001/src/code-stubs-hydrogen...
src/code-stubs-hydrogen.cc:334: is_undetectable_test, graph()->GetConstant0(),
Token::NE);
On 2015/04/30 14:01:08, Toon Verwaest wrote:
> HIsUndetectableAndBranch

Done.

https://codereview.chromium.org/1114563003/diff/40001/src/code-stubs-hydrogen...
src/code-stubs-hydrogen.cc:346: instance_type,
Add<HConstant>(FIRST_NONSTRING_TYPE), Token::LT);
On 2015/04/30 14:01:08, Toon Verwaest wrote:
> <HCheckInstanceType>(string, HCheckInstanceType::IS_STRING)

I didn't do this because there are so many cases I want to check with the
instance type, string being just one of them.

https://codereview.chromium.org/1114563003/diff/40001/src/compiler/js-generic...
File src/compiler/js-generic-lowering.cc (right):

https://codereview.chromium.org/1114563003/diff/40001/src/compiler/js-generic...
src/compiler/js-generic-lowering.cc:278: ReplaceWithStubCall(node, callable,
CallDescriptor::kPatchableCallSite);
On 2015/04/29 08:15:33, Michael Starzinger wrote:
> As discussed offline: If the Typeof-Stub doesn't patch the call-site (the way
> ICs do), we can mark this as CallDescriptor::kNoFlags instead.

Done.

[Toon Verwaest, 2015-05-06 12:36:08]

lgtm

[mvstanton, 2015-05-06 12:49:30]

The CQ bit was checked by mvstanton@chromium.org

[commit-bot: I haz the power, 2015-05-06 12:49:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1114563003/60001

[mvstanton, 2015-05-06 13:06:01]

The CQ bit was unchecked by mvstanton@chromium.org

[mvstanton, 2015-05-06 13:07:07]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2015-05-06 13:07:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/1114563003/#ps80001
(title: "REBASE.")

[commit-bot: I haz the power, 2015-05-06 13:07:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1114563003/80001

[commit-bot: I haz the power, 2015-05-06 13:31:12]

Committed patchset #5 (id:80001)

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

Patchset 5 (id:??) landed as
https://crrev.com/7798548a8f2ef14e8320cc42effa86f0eb8e4294
Cr-Commit-Position: refs/heads/master@{#28260}