Issue 171513003: Don't forget to subtract materialization instructions when computing the offset of the frame pointe…

Issue 171513003: Don't forget to subtract materialization instructions when computing the offset of the frame pointe… (Closed)

Created:
6 years, 10 months ago by turnidge

Modified:
6 years, 10 months ago

Reviewers:
Cutch

CC:
reviews_dartlang.org, vm-dev_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Don't forget to subtract materialization instructions when computing the offset of the frame pointer after deoptimization. R=johnmccutchan@google.com Committed: https://code.google.com/p/dart/source/detail?r=32773

Patch Set 2 : add accessor #

Created: 6 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+18 lines, -7 lines)			Patch
M	runtime/vm/deopt_instructions.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	runtime/vm/object.h	View	1	1 chunk	+3 lines, -0 lines	0 comments	Download
M	runtime/vm/object.cc	View	1	2 chunks	+10 lines, -5 lines	0 comments	Download
M	runtime/vm/stack_frame.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	runtime/vm/stack_frame.cc	View	1	3 chunks	+3 lines, -1 line	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

Cutch

lgtm https://codereview.chromium.org/171513003/diff/1/runtime/vm/stack_frame.cc File runtime/vm/stack_frame.cc (right): https://codereview.chromium.org/171513003/diff/1/runtime/vm/stack_frame.cc#newcode393 runtime/vm/stack_frame.cc:393: num_materializations_ = translation_len - frame_size; Put in an ...

6 years, 10 months ago (2014-02-18 23:44:09 UTC) #3

turnidge

On 2014/02/18 23:36:36, Cutch wrote: > lgtm > > lgtm possible to create a test? ...

6 years, 10 months ago (2014-02-19 00:54:51 UTC) #4

turnidge

https://codereview.chromium.org/171513003/diff/1/runtime/vm/stack_frame.cc File runtime/vm/stack_frame.cc (right): https://codereview.chromium.org/171513003/diff/1/runtime/vm/stack_frame.cc#newcode393 runtime/vm/stack_frame.cc:393: num_materializations_ = translation_len - frame_size; On 2014/02/18 23:44:09, Cutch ...

6 years, 10 months ago (2014-02-19 00:55:53 UTC) #5

turnidge

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

6 years, 10 months ago (2014-02-19 01:02:14 UTC) #6

Florian Schneider

6 years, 10 months ago (2014-02-19 16:24:45 UTC) #7

Message was sent while issue was closed.

On 2014/02/19 00:54:51, turnidge wrote:
> On 2014/02/18 23:36:36, Cutch wrote:
> > lgtm
> > 
> > lgtm possible to create a test?
> 
> I'm not sure how to make a test in a reasonable amount of time here,
> unfortunately.

I'd insert UNREACHABLE at the entry GetDeoptFpOffset to check if this function
is covered by our test at all. I see that two tests use this function:
Debug_InspectStack_Optimized and Debug_InspectStackWithClosure_Optimized. I
would guess that this test may be extended so that it fails if the FP offset
result is not as expected.

Expand Messages | Collapse Messages