881366 - simplify CheckOverRecursed sequence

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Dan Gohman [:sunfish]]

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.

Comment 1

[Dan Gohman [:sunfish]]

Attachment: a proposed fix

Comment 2

[Sean Stangl [: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.

Comment 3

[Dan Gohman [:sunfish]]

(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

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/773df082bd35