First step toward deoptimization using deoptimization info: splits deoptimization into two steps: c…

First step toward deoptimization using deoptimization info: splits deoptimization into two steps: copy frame to buffer, fill unoptimized frame from buffer. The only changes in the frame are the return address and the PC marker. 
Second step will add deoptimization info 'instructions' that will fill the unoptimized frames using the saved registers and frame data.
Committed: https://code.google.com/p/dart/source/detail?r=10071

[siva, 2012-07-31 01:59:12]

couple of questions:

- registers_copy is not being used, why is that?
- unoptimized code might get GCed away once we merge the code heap space and old
space together, shouldn't we be checking for that possibility before switching
to unoptimized code?

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
File runtime/vm/code_generator.cc (right):

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1423: // Access the deopt information for the
deoptimziation point.
deoptimization

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1434: intptr_t* registers_copy = new
intptr_t[kNumberOfCpuRegisters];
ASSERT(saved_registers_address != NULL);
ASSERT(register_copy != NULL);

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1441: DartFrameIterator
iterator(reinterpret_cast<uword>(saved_registers_address));
maybe instead of saved_registers_address you could have a computation of fp and
use that, that would make the code more readable.

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1463: isolate->SetDeoptFrameCopy(frame_copy,
frame_copy_size);
for consistency you should probably set frame_copy in the isolate after copying
all the data in.

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1499: const Code& unoptimized_code =
Code::Handle(function.unoptimized_code());
At this point it is possible that there is no unoptimized code because it has
been GCed, should we be generating it back again over here if it is null.

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1503: intptr_t* frame_copy =
isolate->deopt_frame_copy();
the setter function is named SetDeoptFrameCopy and the getter is named
deopt_frame_copy, can we make them consistent.

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1504: intptr_t* registers_copy =
isolate->deopt_registers_copy();
Where is registers_copy used in this code?

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/isolate.h
File runtime/vm/isolate.h (right):

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/isolate....
runtime/vm/isolate.h:222: void SetDeoptFrameCopy(intptr_t* value, intptr_t size)
{
set_deopt_frame_copy to be consistent?

[srdjan, 2012-07-31 16:21:28]

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
File runtime/vm/code_generator.cc (right):

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1423: // Access the deopt information for the
deoptimziation point.
On 2012/07/31 01:59:12, asiva wrote:
> deoptimization

Done.

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1434: intptr_t* registers_copy = new
intptr_t[kNumberOfCpuRegisters];
On 2012/07/31 01:59:12, asiva wrote:
> ASSERT(saved_registers_address != NULL);
> ASSERT(register_copy != NULL);

Done.

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1441: DartFrameIterator
iterator(reinterpret_cast<uword>(saved_registers_address));
On 2012/07/31 01:59:12, asiva wrote:
> maybe instead of saved_registers_address you could have a computation of fp
and
> use that, that would make the code more readable.

Done.

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1463: isolate->SetDeoptFrameCopy(frame_copy,
frame_copy_size);
On 2012/07/31 01:59:12, asiva wrote:
> for consistency you should probably set frame_copy in the isolate after
copying
> all the data in.

Done.

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1499: const Code& unoptimized_code =
Code::Handle(function.unoptimized_code());
On 2012/07/31 01:59:12, asiva wrote:
> At this point it is possible that there is no unoptimized code because it has
> been GCed, should we be generating it back again over here if it is null.

Function always refers to unoptimized code, i.e., we would need to implement it
as weak reference. Once that is in we can check for unoptimized code being
removed, but that means that we could have GC at this point, and we cannot do
that. The ASSERT below tests that the unoptimized code exists.

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1503: intptr_t* frame_copy =
isolate->deopt_frame_copy();
On 2012/07/31 01:59:12, asiva wrote:
> the setter function is named SetDeoptFrameCopy and the getter is named
> deopt_frame_copy, can we make them consistent. 

SetDeoptFrameCopy sets the size and the buffer, it is not a simple setter.

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/code_gen...
runtime/vm/code_generator.cc:1504: intptr_t* registers_copy =
isolate->deopt_registers_copy();
On 2012/07/31 01:59:12, asiva wrote:
> Where is registers_copy used in this code?

Current deoptimization stubs already save the registers in right place. In next
step the registers will be save into unoptimized frame using registers_copy_

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/isolate.h
File runtime/vm/isolate.h (right):

https://chromiumcodereview.appspot.com/10836020/diff/5003/runtime/vm/isolate....
runtime/vm/isolate.h:222: void SetDeoptFrameCopy(intptr_t* value, intptr_t size)
{
On 2012/07/31 01:59:12, asiva wrote:
> set_deopt_frame_copy to be consistent?

This is not a simple setter, therefore it cannot be named  set_deopt_frame_copy

[siva, 2012-07-31 17:19:23]

Thanks for the explanations.

LGTM.