1407867 - mips64 save/restore register fp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[yuyin]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/604.1.38 (KHTML, like Gecko) Version/11.0 Safari/604.1.38

Steps to reproduce:

pull gecko, change to branch inbound.




Actual results:

some jit-test failed on mips64.


Expected results:

all pass.

Comment 1

[yuyin]

fp use as Assembler::FramePointer, Bug 1393723 save/restore it in trampoline only on mips32, add support on mips64.

Comment 2

[yuyin]

Attachment: 0001-MIPS64-Add-missing-save-restore-registe-fp.patch

Comment 3

[yuyin]

Attachment: 0001-MIPS64-Add-missing-save-restore-registe-fp.patch

Comment 4

[yuyin]

sorry， two attach 8917630 and  8917632 are same, Submitted two times

Comment 5

[Lars T Hansen [:lth]]

Comment on attachment 8917632 [details] [diff] [review]
0001-MIPS64-Add-missing-save-restore-registe-fp.patch

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

::: js/src/jit/mips64/Trampoline-mips64.cpp
@@ +44,4 @@
>      double f25;
>      double f24;
>  
> +    uintptr_t align;

Unused declaration?

Comment 6

[yuyin]

Thank you.

I want change it to :

--------------
 MOZ_MAYBE_UNUSED uintptr_t align;
--------------

what about do you think.

(In reply to Lars T Hansen [:lth] from comment #5)
> Comment on attachment 8917632 [details] [diff] [review]
> 0001-MIPS64-Add-missing-save-restore-registe-fp.patch
> 
> Review of attachment 8917632 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jit/mips64/Trampoline-mips64.cpp
> @@ +44,4 @@
> >      double f25;
> >      double f24;
> >  
> > +    uintptr_t align;
> 
> Unused declaration?

Comment 7

[Lars T Hansen [:lth]]

Whatever works for you is fine with me.  I was just curious about it because i didn't see a use of the variable in the patch.  But perhaps you are using it elsewhere and I'm just not aware of it?

Comment 8

[Lars T Hansen [:lth]]

Oh, I see - it is padding for the structure.  So you should ignore my comment.

Comment 9

[yuyin]

thank you for review :)

(In reply to Lars T Hansen [:lth] from comment #8)
> Oh, I see - it is padding for the structure.  So you should ignore my
> comment.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2eb7a581bde
MIPS64: Add missing save/restore registe fp. r=lth

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e2eb7a581bde