Cleanup more isolate usage in ia32 files.

Cleanup more isolate usage in ia32 files.

Committed: http://code.google.com/p/v8/source/detail?r=7367

[Vitaly Repeshko, 2011-03-25 12:57:51]

LGTM!

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:134: HandleScope scope;
Is this really needed?

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:137: if (!function->IsOptimized()) return;
I don't know how often this happens, but it still should be the first line in
the function.

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:193: Isolate* isolate = code->GetIsolate();
Move this up and use the isolate in HandleScope if the scope required.

http://codereview.chromium.org/6696107/diff/1/src/ia32/stub-cache-ia32.cc
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/6696107/diff/1/src/ia32/stub-cache-ia32.cc#new...
src/ia32/stub-cache-ia32.cc:920: Heap* heap = masm()->isolate()->heap();
There's StubCompiler::isolate() that we could use in its member functions like
this one.

[Mads Ager (chromium), 2011-03-25 13:09:26]

Thanks Vitaly.

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:134: HandleScope scope;
On 2011/03/25 12:57:51, Vitaly Repeshko wrote:
> Is this really needed?

No you are right. That is not needed. I have extracted the isolate earlier in
the method above and passed it to the handlescope constructor.

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:137: if (!function->IsOptimized()) return;
On 2011/03/25 12:57:51, Vitaly Repeshko wrote:
> I don't know how often this happens, but it still should be the first line in
> the function.

Done.

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:193: Isolate* isolate = code->GetIsolate();
On 2011/03/25 12:57:51, Vitaly Repeshko wrote:
> Move this up and use the isolate in HandleScope if the scope required.

Done.

http://codereview.chromium.org/6696107/diff/1/src/ia32/stub-cache-ia32.cc
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/6696107/diff/1/src/ia32/stub-cache-ia32.cc#new...
src/ia32/stub-cache-ia32.cc:920: Heap* heap = masm()->isolate()->heap();
On 2011/03/25 12:57:51, Vitaly Repeshko wrote:
> There's StubCompiler::isolate() that we could use in its member functions like
> this one.

Great! Replaced a lot of occurences of masm->isolate() by isolate().

[Mads Ager (chromium), 2011-03-25 13:15:52]

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:134: HandleScope scope;
On 2011/03/25 13:09:26, Mads Ager wrote:
> On 2011/03/25 12:57:51, Vitaly Repeshko wrote:
> > Is this really needed?
> 
> No you are right. That is not needed. I have extracted the isolate earlier in
> the method above and passed it to the handlescope constructor.

I was wrong. We need it. The code patcher allocates handles to put in the code
and there is no other handle scope here. I'll get the isolate early and use it
for handle scope construction.

[Vitaly Repeshko, 2011-03-25 13:25:03]

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:134: HandleScope scope;
On 2011/03/25 13:15:53, Mads Ager wrote:
> On 2011/03/25 13:09:26, Mads Ager wrote:
> > On 2011/03/25 12:57:51, Vitaly Repeshko wrote:
> > > Is this really needed?
> > 
> > No you are right. That is not needed. I have extracted the isolate earlier
in
> > the method above and passed it to the handlescope constructor.
> 
> I was wrong. We need it. The code patcher allocates handles to put in the code
> and there is no other handle scope here. I'll get the isolate early and use it
> for handle scope construction.

Is it only the self-reference code handle or are there more?

[Mads Ager (chromium), 2011-03-25 13:37:42]

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:134: HandleScope scope;
On 2011/03/25 13:25:03, Vitaly Repeshko wrote:
> On 2011/03/25 13:15:53, Mads Ager wrote:
> > On 2011/03/25 13:09:26, Mads Ager wrote:
> > > On 2011/03/25 12:57:51, Vitaly Repeshko wrote:
> > > > Is this really needed?
> > > 
> > > No you are right. That is not needed. I have extracted the isolate earlier
> in
> > > the method above and passed it to the handlescope constructor.
> > 
> > I was wrong. We need it. The code patcher allocates handles to put in the
code
> > and there is no other handle scope here. I'll get the isolate early and use
it
> > for handle scope construction.
> 
> Is it only the self-reference code handle or are there more?

That is the only one that I found by quick inspection. With the isolate
extracted early the cost of the handle scope is very little compared to the rest
of the work here.

[Vitaly Repeshko, 2011-03-25 13:45:21]

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/6696107/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:134: HandleScope scope;
On 2011/03/25 13:37:42, Mads Ager wrote:
> On 2011/03/25 13:25:03, Vitaly Repeshko wrote:
> > On 2011/03/25 13:15:53, Mads Ager wrote:
> > > On 2011/03/25 13:09:26, Mads Ager wrote:
> > > > On 2011/03/25 12:57:51, Vitaly Repeshko wrote:
> > > > > Is this really needed?
> > > > 
> > > > No you are right. That is not needed. I have extracted the isolate
earlier
> > in
> > > > the method above and passed it to the handlescope constructor.
> > > 
> > > I was wrong. We need it. The code patcher allocates handles to put in the
> code
> > > and there is no other handle scope here. I'll get the isolate early and
use
> it
> > > for handle scope construction.
> > 
> > Is it only the self-reference code handle or are there more?
> 
> That is the only one that I found by quick inspection. With the isolate
> extracted early the cost of the handle scope is very little compared to the
rest
> of the work here.

Sure. The performance is not a concern here. I just was worried that we have
hidden handlified code that may allocate when not caught by the debug mode
AssertNoAllocation.