With >= 64 non-string instance types, I removed an optimization

With >= 64 non-string instance types, I removed an optimization that allowed us to detect internalized strings with a single bit test. (https://code.google.com/p/v8/source/detail?r=15358) But that change caused a regression, so here is an improvement:

Put the internalized string types in the lower 64 entries of 
INSTANCE_TYPE, and non-internalized string types in the next 64 entries.
This way we can restore the single bit check.

BUG=
R=yangguo@chromium.org

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

[mvstanton, 2013-07-19 09:20:29]

Hi Yang,
Here is the internalized string instance type change we discussed. Thanks for
having a look!
--Michael

[Yang, 2013-07-19 09:45:06]

lgtm with comment.

https://codereview.chromium.org/19749004/diff/2001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/19749004/diff/2001/src/objects.h#newcode640
src/objects.h:640: INTERNALIZED_STRING_TYPE = kTwoByteStringTag | kSeqStringTag,
I would prefer to have the kInternalizedTag added here as well, otherwise we are
implicating that kInternalizedTag == 0.

Same for all the other ones. It's a bit convoluted, but we only do it this once
here.

[Yang, 2013-07-19 09:52:18]

https://codereview.chromium.org/19749004/diff/2001/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/19749004/diff/2001/src/ia32/code-stubs-ia32.c...
src/ia32/code-stubs-ia32.cc:4116: __ cmp(scratch, kStringTag |
kInternalizedTag);
change that to a test, saving one instruction.

https://codereview.chromium.org/19749004/diff/2001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/19749004/diff/2001/src/objects-inl.h#newcode225
src/objects-inl.h:225: return type < kNotInternalizedTag;
I kinda prefer the bit-masking way, since it doesn't implicitly assume anything.
Shouldn't be more expensive. Below as well. But this is your choice.

[mvstanton, 2013-07-19 11:07:41]

Thanks for the review Yang, I updated per your points.
All the best,
--Michael

https://codereview.chromium.org/19749004/diff/2001/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/19749004/diff/2001/src/ia32/code-stubs-ia32.c...
src/ia32/code-stubs-ia32.cc:4116: __ cmp(scratch, kStringTag |
kInternalizedTag);
On 2013/07/19 09:52:18, Yang wrote:
> change that to a test, saving one instruction.

Done. It's done on x64 as well (ARM is formulated differently).

https://codereview.chromium.org/19749004/diff/2001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/19749004/diff/2001/src/objects-inl.h#newcode225
src/objects-inl.h:225: return type < kNotInternalizedTag;
On 2013/07/19 09:52:18, Yang wrote:
> I kinda prefer the bit-masking way, since it doesn't implicitly assume
anything.
> Shouldn't be more expensive. Below as well. But this is your choice.

I wondered about that too, it's better to be symmetrical with the way we do the
test in assembly. I'll change to the bit test.

https://codereview.chromium.org/19749004/diff/2001/src/objects-inl.h#newcode322
src/objects-inl.h:322: return type_ < kNotInternalizedTag;
I restored this one too.

https://codereview.chromium.org/19749004/diff/2001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/19749004/diff/2001/src/objects.h#newcode640
src/objects.h:640: INTERNALIZED_STRING_TYPE = kTwoByteStringTag | kSeqStringTag,
On 2013/07/19 09:45:06, Yang wrote:
> I would prefer to have the kInternalizedTag added here as well, otherwise we
are
> implicating that kInternalizedTag == 0.
> 
> Same for all the other ones. It's a bit convoluted, but we only do it this
once
> here.

Done.

[mvstanton, 2013-07-19 11:29:20]

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