Closed Bug 681698 Opened 13 years ago Closed 13 years ago

IonMonkey: Rename Snapshot

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: dvander)

Details

Attachments

(1 file)

The word "Snapshot" has like three meanings which is too many:
 (1) MIR snapshots which capture stack state at resume points.
 (2) LIR snapshots which capture frame state at bailout points.
 (3) Compressed versions of (2) which are attached to scripts.

(2) and (3) seem related enough.

Proposal: rename (1) to MSafePoint or maybe MResumePoint?
I'm +1 on ResumePoint.
Attached patch patchSplinter Review
MResumePoint it is!
Attachment #557738 - Flags: review?(sstangl)
Comment on attachment 557738 [details] [diff] [review]
patch

Review of attachment 557738 [details] [diff] [review]:
-----------------------------------------------------------------

r+'d with gusto.
Attachment #557738 - Flags: review?(sstangl) → review+
Comment on attachment 557738 [details] [diff] [review]
patch

Review of attachment 557738 [details] [diff] [review]:
-----------------------------------------------------------------

The Big Honkin' comment is easier to understand. ;)
Attachment #557738 - Flags: feedback+
(In reply to David Anderson [:dvander] from comment #2)
> Created attachment 557738 [details] [diff] [review]
> patch
> 
> MResumePoint it is!


I wonder if a MInstruction such as MCall will not always have a resumePoint linked to it (just by looking at CodeGenerator function bodies).  Unless we need to have return points only for a sub-set of all handled architectures we could move the resume point construction inside MInstructions.
(In reply to Nicolas B. Pierron [:pierron] from comment #4)
Glad to hear it!

(In reply to Nicolas B. Pierron [:pierron] from comment #5)
> I wonder if a MInstruction such as MCall will not always have a resumePoint

Anything that is not idempotent must have a resume point attached, so it's hard to see how we could meaningfully avoid this for MCall (TI might be able to, but then we should be probably be inlining the function anyway).

http://hg.mozilla.org/projects/ionmonkey/rev/c7199a1523c3
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.