simplify CheckOverRecursed sequence

RESOLVED FIXED in mozilla24

Status

()

--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sunfish, Unassigned)

Tracking

Trunk
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
The CheckOverRecursed sequence currently loads the limit value into a register and then does a comparison and branch on it. On platforms like x86, the load can be folded into the comparison.
(Reporter)

Comment 1

5 years ago
Created attachment 760501 [details] [diff] [review]
a proposed fix
Attachment #760501 - Flags: review?(sstangl)
Comment on attachment 760501 [details] [diff] [review]
a proposed fix

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

In jscntxt.cpp:1084, there's a comment that reads "IonMonkey sets its stack limit to NULL to trigger operation callbacks."

Would you mind changing this comment to "...limit to UINTPTR_MAX to..." as part of this patch?

::: js/src/ion/CodeGenerator.cpp
@@ +2146,5 @@
>      if (!addOutOfLineCode(ool))
>          return false;
>  
>      // Conditional forward (unlikely) branch to failure.
> +    masm.branchPtr(Assembler::Above, AbsoluteAddress(limitAddr), StackPointer,

AboveOrEqual?

@@ +2147,5 @@
>          return false;
>  
>      // Conditional forward (unlikely) branch to failure.
> +    masm.branchPtr(Assembler::Above, AbsoluteAddress(limitAddr), StackPointer,
> +                   ool->entry());

Coding style has 100 columns.
Attachment #760501 - Flags: review?(sstangl) → review+
(Reporter)

Comment 3

5 years ago
(In reply to Sean Stangl [:sstangl] from comment #2)
> Comment on attachment 760501 [details] [diff] [review]
> a proposed fix
> 
> Review of attachment 760501 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> In jscntxt.cpp:1084, there's a comment that reads "IonMonkey sets its stack
> limit to NULL to trigger operation callbacks."
> 
> Would you mind changing this comment to "...limit to UINTPTR_MAX to..." as
> part of this patch?

Done.

> ::: js/src/ion/CodeGenerator.cpp
> @@ +2146,5 @@
> >      if (!addOutOfLineCode(ool))
> >          return false;
> >  
> >      // Conditional forward (unlikely) branch to failure.
> > +    masm.branchPtr(Assembler::Above, AbsoluteAddress(limitAddr), StackPointer,
> 
> AboveOrEqual?

Oh yes, good catch.

> @@ +2147,5 @@
> >          return false;
> >  
> >      // Conditional forward (unlikely) branch to failure.
> > +    masm.branchPtr(Assembler::Above, AbsoluteAddress(limitAddr), StackPointer,
> > +                   ool->entry());
> 
> Coding style has 100 columns.

Fixed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/773df082bd35

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/773df082bd35
Status: UNCONFIRMED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.