Closed Bug 476716 Opened 13 years ago Closed 13 years ago

Unmacroize ip_adj helpers.

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jorendorff, Assigned: jorendorff)

Details

(Keywords: fixed1.9.1, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 1 obsolete file)

Attached patch v1 (obsolete) — Splinter Review
I did this in the process of debugging bug 462027 comment 54, and I'd like to commit the change.
Attachment #360365 - Flags: review?(brendan)
Need to get this into 1.9.1 to avoid merge hell.

/be
Flags: wanted1.9.1?
Comment on attachment 360365 [details] [diff] [review]
v1

The NAMING_CONVENTION shouts MACRO! but that's ok -- can clean up later, or leave as a warning and a mystery ;-).

/be
Attachment #360365 - Flags: review?(brendan) → review+
Comment on attachment 360365 [details] [diff] [review]
v1

>+FI_IMACRO_PC(const FrameInfo &fi, JSStackFrame *fp)
>+{
>+    if (IMACRO_PC_ADJ(fi.ip_adj))
>+        return imacro_code[*FI_SCRIPT_PC(fi, fp)] + IMACRO_PC_ADJ(fi.ip_adj);
>+    return NULL;
>+}
[...]
>+DECODE_IP_ADJ(uintptr_t ip, JSStackFrame *fp)
>+{
>+    if (IMACRO_PC_ADJ(ip)) {
>+        fp->imacpc = fp->script->code + SCRIPT_PC_ADJ(ip);
>+        fp->regs->pc = imacro_code[*fp->imacpc] + IMACRO_PC_ADJ(ip);

Now that these are inlines, would it be clearer to avoid repeating calls and counting on CSE, and use a local var to capture the result of one call to IMACRO_PC_ADJ(ip)?

/be
Attached patch v2Splinter Review
Completely different patch.

A minor drawback with this approach is that the numbers in the debug spew for LIR guard instructions are now pointers, not bytecode offsets:

    xf1: xf eq1 -> 0x30db81:0x0 sp+0 rp+0

I can live with it.
Assignee: general → jorendorff
Attachment #360365 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #361607 - Flags: review?(brendan)
Comment on attachment 361607 [details] [diff] [review]
v2

Great! Bye-byte imacro_code. Thanks,

/be
Attachment #361607 - Flags: review?(brendan) → review+
http://hg.mozilla.org/tracemonkey/rev/ab8216b1e596
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Reopening because this is nominated for 1.9.1 (nothing wrong with the patch, which is still in tracemonkey).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: fixed-in-tracemonkey
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Flags: wanted1.9.1? → wanted1.9.1+
Resolution: --- → FIXED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.