Closed Bug 1453628 Opened 6 years ago Closed 6 years ago

[MIPS64] Cleanup JIT <-> C++ 32-bit argument passing for simulator build

Categories

(Core :: JavaScript Engine: JIT, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: dragan.mladjenovic, Assigned: dragan.mladjenovic)

Details

Attachments

(1 file, 1 obsolete file)

      No description provided.
Summary: [MIPS64] Cleanup jit <-> c++ 32-bit argument passing for simulator builds → [MIPS64] Cleanup JIT <-> C++ 32-bit argument passing for simulator build
Attached patch bug1453628.patch (obsolete) — Splinter Review
Changes CALL_GENERATED_* macros to extend its arguments to full pointer with which ensures that high bits of int32_t are not garbage when passed from C++ to simulator. Does not correctly provide sign extension of uint32_t as per mips64 ABI though, but luckily we don't have uint32_t values with high bit set floating around. Also removes workarounds and and provides correct type declaration for external function prototypes (Prototype_*) in simulator.
Assignee: nobody → dragan.mladjenovic
Attachment #8967334 - Flags: review?(bbouvier)
Comment on attachment 8967334 [details] [diff] [review]
bug1453628.patch

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

Good. Just to make sure this is safe on other simulator platforms (including ARM and ARM64), could you try-run this patch, please? Thanks!

::: js/src/jit/mips64/Simulator-mips64.cpp
@@ +2142,2 @@
>  
> +typedef double (*Prototype_DoubleInt)(double arg0, int32_t arg1);

pre-existing nit: this name should be Prototype_Double_DoubleInt, if I follow the logic around?

(might be the case in other backends too, including ARM simulator)

@@ +2347,5 @@
>            }
>            case Args_Int_IntDouble: {
>              double dval1 = getFpuRegisterDouble(13);
>              Prototype_Int_IntDouble target = reinterpret_cast<Prototype_Int_IntDouble>(external);
> +            int64_t result = target(int32_t(arg0), dval1);

nit: the result should be int32
Attachment #8967334 - Flags: review?(bbouvier) → review+
Attachment #8967334 - Attachment is obsolete: true
Attachment #8968155 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f88b7fb02d30
[MIPS64] Cleanup JIT <-> C++ 32-bit argument passing for simulator build. r=bbouvier
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f88b7fb02d30
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.