Last Comment Bug 698563 - IonMonkey: rather than generating nothing, generateArgumentsRectifier should generate an ArgumentsRectifier.
: IonMonkey: rather than generating nothing, generateArgumentsRectifier should ...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: ARM Linux
: -- normal (vote)
: ---
Assigned To: general
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-31 13:21 PDT by Marty Rosenberg [:mjrosenb]
Modified: 2011-12-01 17:31 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Write the body of the arguments rectifier. (11.82 KB, patch)
2011-11-09 12:30 PST, Marty Rosenberg [:mjrosenb]
Jacob.Bramley: review+
Details | Diff | Splinter Review

Description Marty Rosenberg [:mjrosenb] 2011-10-31 13:21:43 PDT

    
Comment 1 Marty Rosenberg [:mjrosenb] 2011-11-09 12:30:26 PST
Created attachment 573289 [details] [diff] [review]
Write the body of the arguments rectifier.

This was a pretty straightforward translation of the x64 variant.
Comment 2 Jacob Bramley [:jbramley] 2011-11-14 07:25:38 PST
Comment on attachment 573289 [details] [diff] [review]
Write the body of the arguments rectifier.

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

One niggle to be addressed, but otherwise good.

::: js/src/ion/arm/MacroAssembler-arm.cpp
@@ +704,5 @@
>                            Index mode, Assembler::Condition cc)
>  {
> +    int x = offset.value;
> +    // we can encode this as a standard ldr... MAKE IT SO
> +    if (size == 32 || size == 8 && !IsSigned ) {

That would be clearer with brackets:

if (size == 32 || (size == 8 && !IsSigned))

(I had to look up the || versus && precedence to check it was correct.)

::: js/src/ion/arm/Trampoline-arm.cpp
@@ +219,2 @@
>  
> +    masm.ma_add(r3, lsl(r8, 3), r3); // r3 <- r3 + nargs * 8

Ooh, that is elegant. I like it :-)

Note You need to log in before you can comment on or make changes to this bug.