Closed
Bug 1020359
Opened 10 years ago
Closed 10 years ago
IonMonkey MIPS: Fix build issues for MIPS caused by latest updates
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: rankov, Assigned: rankov)
Details
Attachments
(7 files, 2 obsolete files)
1.18 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
6.39 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
6.00 KB,
patch
|
rankov
:
review+
|
Details | Diff | Splinter Review |
1.81 KB,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
6.26 KB,
patch
|
rankov
:
review+
|
Details | Diff | Splinter Review |
1.12 KB,
patch
|
shu
:
review+
|
Details | Diff | Splinter Review |
4.61 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
MIPS build fails due to latest changes. Some of these changes origin from bug 1017539.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → branislav.rankov
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Comment 3•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•10 years ago
|
Attachment #8435015 -
Flags: review?(jdemooij)
Assignee | ||
Updated•10 years ago
|
Attachment #8435016 -
Flags: review?(jdemooij)
Assignee | ||
Updated•10 years ago
|
Attachment #8435017 -
Flags: review?(jdemooij)
Updated•10 years ago
|
Attachment #8435015 -
Flags: review?(jdemooij) → review+
Comment 4•10 years ago
|
||
Comment on attachment 8435016 [details] [diff] [review] 02-Fix-StoreSlotT.patch Review of attachment 8435016 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips/MacroAssembler-mips.cpp @@ +2703,5 @@ > > +template <typename T> > +void > +MacroAssemblerMIPSCompat::storeUnboxedValue(ConstantOrRegister value, MIRType valueType, const T &dest, > + MIRType slotType) Nit: add an extra space here so that the arguments line up. @@ +2723,5 @@ > +} > + > +template void > +MacroAssemblerMIPSCompat::storeUnboxedValue(ConstantOrRegister value, MIRType valueType, const Address &dest, > + MIRType slotType); Same here and below.
Attachment #8435016 -
Flags: review?(jdemooij) → review+
Updated•10 years ago
|
Attachment #8435017 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Fixed the nits. Carry review from previous patch.
Attachment #8435016 -
Attachment is obsolete: true
Attachment #8437634 -
Flags: review+
Assignee | ||
Comment 6•10 years ago
|
||
Here is the try link for the patches: https://tbpl.mozilla.org/?tree=Try&rev=2767a212b2be
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4dc51e7f8e8d https://hg.mozilla.org/integration/mozilla-inbound/rev/0e034ef0b7b9 https://hg.mozilla.org/integration/mozilla-inbound/rev/f441afd30e52
Keywords: leave-open
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8449508 -
Flags: review?(nicolas.b.pierron)
Assignee | ||
Comment 9•10 years ago
|
||
This is follow-up on bug 1020359.
Attachment #8449513 -
Flags: review?(nicolas.b.pierron)
https://hg.mozilla.org/mozilla-central/rev/4dc51e7f8e8d https://hg.mozilla.org/mozilla-central/rev/0e034ef0b7b9 https://hg.mozilla.org/mozilla-central/rev/f441afd30e52
Updated•10 years ago
|
Attachment #8449508 -
Flags: review?(nicolas.b.pierron) → review+
Comment 11•10 years ago
|
||
Comment on attachment 8449513 [details] [diff] [review] 05-pjs-bailout.patch Review of attachment 8449513 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips/Trampoline-mips.cpp @@ +523,5 @@ > return code; > } > > +// NOTE: Members snapshotOffset_ and padding_ of BailoutStack > +// are not stored in this function. nit: "this function" ? @@ +630,5 @@ > + PushBailoutFrame(masm, frameClass, a0); > + > + // Parallel bailout is like parallel failure in that we unwind all the way > + // to the entry frame. Reserve space for the frame pointer of the entry frame. > + const int sizeOfEntryFramePointer = sizeof(uint8_t *) * 2; shu: Why "* 2" if this is a pointer?
Attachment #8449513 -
Flags: review?(nicolas.b.pierron) → review?(shu)
Comment 12•10 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #11) > Comment on attachment 8449513 [details] [diff] [review] > 05-pjs-bailout.patch > > Review of attachment 8449513 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: js/src/jit/mips/Trampoline-mips.cpp > @@ +523,5 @@ > > return code; > > } > > > > +// NOTE: Members snapshotOffset_ and padding_ of BailoutStack > > +// are not stored in this function. > > nit: "this function" ? > > @@ +630,5 @@ > > + PushBailoutFrame(masm, frameClass, a0); > > + > > + // Parallel bailout is like parallel failure in that we unwind all the way > > + // to the entry frame. Reserve space for the frame pointer of the entry frame. > > + const int sizeOfEntryFramePointer = sizeof(uint8_t *) * 2; > > shu: Why "* 2" if this is a pointer? I confess this is blindly copied from things like [1], which is also an outparam pointer. I figured it was for alignment reasons, but I don't know for sure. http://dxr.mozilla.org/mozilla-central/source/js/src/jit/mips/Trampoline-mips.cpp#548
Comment 13•10 years ago
|
||
Comment on attachment 8449513 [details] [diff] [review] 05-pjs-bailout.patch Review of attachment 8449513 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/mips/Trampoline-mips.cpp @@ +639,5 @@ > + masm.passABIArg(a0); > + masm.passABIArg(a1); > + masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, BailoutPar)); > + > + // Get the frame pointer of the entry fram and return. Existing typo: frame. Do you mind fixing this in Trampoline-arm.cpp as well?
Attachment #8449513 -
Flags: review?(shu) → review+
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Shu-yu Guo [:shu] from comment #12) > I confess this is blindly copied from things like [1], which is also an > outparam pointer. I figured it was for alignment reasons, but I don't know > for sure. > > http://dxr.mozilla.org/mozilla-central/source/js/src/jit/mips/Trampoline- > mips.cpp#548 You are right. The size of bailoutInfoOutParamSize is 2*sizeof(uintptr_t) because of alignment. (In reply to Shu-yu Guo [:shu] from comment #13) > ::: js/src/jit/mips/Trampoline-mips.cpp > @@ +639,5 @@ > > + masm.passABIArg(a0); > > + masm.passABIArg(a1); > > + masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, BailoutPar)); > > + > > + // Get the frame pointer of the entry fram and return. > > Existing typo: frame. Do you mind fixing this in Trampoline-arm.cpp as well? Sure. I'll make another patch for this.
Assignee | ||
Comment 15•10 years ago
|
||
Fixed nits. Carry review from previous patch.
Attachment #8449513 -
Attachment is obsolete: true
Attachment #8450944 -
Flags: review+
Assignee | ||
Comment 16•10 years ago
|
||
Attachment #8450946 -
Flags: review?(shu)
Comment 17•10 years ago
|
||
Comment on attachment 8450946 [details] [diff] [review] 06-arm-typo.patch Review of attachment 8450946 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8450946 -
Flags: review?(shu) → review+
Assignee | ||
Comment 18•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=1b62b18ec9fa https://hg.mozilla.org/integration/mozilla-inbound/rev/41c2396b3761 https://hg.mozilla.org/integration/mozilla-inbound/rev/f221afb93d0b https://hg.mozilla.org/integration/mozilla-inbound/rev/4baddb2df1db
https://hg.mozilla.org/mozilla-central/rev/41c2396b3761 https://hg.mozilla.org/mozilla-central/rev/f221afb93d0b https://hg.mozilla.org/mozilla-central/rev/4baddb2df1db
Assignee | ||
Comment 20•10 years ago
|
||
Attachment #8465469 -
Flags: review?(jdemooij)
Updated•10 years ago
|
Attachment #8465469 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 21•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6f981a6431a
Assignee | ||
Updated•10 years ago
|
Keywords: leave-open
Comment 22•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a6f981a6431a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Updated•10 years ago
|
QA Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•