Fix logic error in assert in IsUndeclaredGlobal()

Fix logic error in assert in IsUndeclaredGlobal()

Recent changes in IC logic meant that CallStubs no longer use the Contextual bit. IsUndeclaredGlobal() needed to adjust for that.

In fact, now the CL has morphed to remove the notion of storing contextual state in the IC at all, it just becomes some extra ic state of the load ic. This took some adjustment in harmony code to use the global receiver for certain stores.

Now it's clearer that only LoadICs actually record any information about contextual or not.

R=verwaest@chromium.org

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

[mvstanton, 2014-01-17 09:47:51]

Hi Toon,
Here is the fix for the issue introduced by r18594
(https://code.google.com/p/v8/source/detail?r=18594).

The fix itself is 1-2 lines, but per our discussion we used this opportunity to
go ahead and devolve Contextual() to a property of LoadICs. Very fun! :)

The regression test asserts in debug without the change. PTAL.
--Michael

https://codereview.chromium.org/140943002/diff/30001/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/140943002/diff/30001/src/code-stubs.h#newcode...
src/code-stubs.h:1091: STATIC_ASSERT(LoadIC::Contextual::kShift ==
ContextualBits::kShift);
Toon, any insight into whats going on here? This appears to be a usage of
contextual information in a call ic.

https://codereview.chromium.org/140943002/diff/30001/src/full-codegen.cc
File src/full-codegen.cc (left):

https://codereview.chromium.org/140943002/diff/30001/src/full-codegen.cc#oldc...
src/full-codegen.cc:455: Handle<Code> ic = StoreIC::initialize_stub(isolate(),
strict_mode(), mode);
In a follow-on CL I could eliminate the mode argument to this function, and
eliminate the mode argument down into CallIC as well.

[Toon Verwaest, 2014-01-17 10:05:55]

https://codereview.chromium.org/140943002/diff/30001/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/140943002/diff/30001/src/code-stubs.h#newcode...
src/code-stubs.h:1091: STATIC_ASSERT(LoadIC::Contextual::kShift ==
ContextualBits::kShift);
This should be removed as well. This is a call IC which used to contain the
contextual bit; which isn't the case anymore.

On 2014/01/17 09:47:51, mvstanton wrote:
> Toon, any insight into whats going on here? This appears to be a usage of
> contextual information in a call ic.

https://codereview.chromium.org/140943002/diff/80001/src/ia32/lithium-codegen...
File src/ia32/lithium-codegen-ia32.cc (left):

https://codereview.chromium.org/140943002/diff/80001/src/ia32/lithium-codegen...
src/ia32/lithium-codegen-ia32.cc:3207: CONTEXTUAL);
So now you should be able to remove StoreGlobalGeneric as well, given that it's
the same as StoreNamedGeneric. You can do this in a follow-up CL if you prefer.

https://codereview.chromium.org/140943002/diff/80001/src/stub-cache.cc
File src/stub-cache.cc (right):

https://codereview.chromium.org/140943002/diff/80001/src/stub-cache.cc#newcod...
src/stub-cache.cc:891: if (!ic.IsContextualLoad()) {
I think you can instantiate a LoadIC here and move IsContextual fully into
LoadIC.

[mvstanton, 2014-01-17 10:44:33]

Comments addressed, thx, ptal and I'll make some ports.
--Michael

https://codereview.chromium.org/140943002/diff/30001/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/140943002/diff/30001/src/code-stubs.h#newcode...
src/code-stubs.h:1091: STATIC_ASSERT(LoadIC::Contextual::kShift ==
ContextualBits::kShift);
On 2014/01/17 10:05:56, Toon Verwaest wrote:
> This should be removed as well. This is a call IC which used to contain the
> contextual bit; which isn't the case anymore.
> 
> On 2014/01/17 09:47:51, mvstanton wrote:
> > Toon, any insight into whats going on here? This appears to be a usage of
> > contextual information in a call ic.
> 


Done. I kept asserts that indicate bit 0 is "busy," so we start at bit 1.

https://codereview.chromium.org/140943002/diff/80001/src/ia32/lithium-codegen...
File src/ia32/lithium-codegen-ia32.cc (left):

https://codereview.chromium.org/140943002/diff/80001/src/ia32/lithium-codegen...
src/ia32/lithium-codegen-ia32.cc:3207: CONTEXTUAL);
On 2014/01/17 10:05:56, Toon Verwaest wrote:
> So now you should be able to remove StoreGlobalGeneric as well, given that
it's
> the same as StoreNamedGeneric. You can do this in a follow-up CL if you
prefer.

Yowza! Let's do-it-to-it.

https://codereview.chromium.org/140943002/diff/80001/src/stub-cache.cc
File src/stub-cache.cc (right):

https://codereview.chromium.org/140943002/diff/80001/src/stub-cache.cc#newcod...
src/stub-cache.cc:891: if (!ic.IsContextualLoad()) {
On 2014/01/17 10:05:56, Toon Verwaest wrote:
> I think you can instantiate a LoadIC here and move IsContextual fully into
> LoadIC.

Good idea, done.

[Toon Verwaest, 2014-01-17 10:53:45]

lgtm with nit

https://codereview.chromium.org/140943002/diff/160001/src/ic.h
File src/ic.h (right):

https://codereview.chromium.org/140943002/diff/160001/src/ic.h#newcode118
src/ic.h:118: bool IsLoadStub() const {
You can move this back into the #debug section.

[mvstanton, 2014-01-17 11:08:12]

thx! ports are boilerplate. Will commit.

https://codereview.chromium.org/140943002/diff/160001/src/ic.h
File src/ic.h (right):

https://codereview.chromium.org/140943002/diff/160001/src/ic.h#newcode118
src/ic.h:118: bool IsLoadStub() const {
On 2014/01/17 10:53:45, Toon Verwaest wrote:
> You can move this back into the #debug section.

Done.

[mvstanton, 2014-01-17 11:08:42]

Committed patchset #5 manually as r18660 (presubmit successful).