Status

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: m_kato, Assigned: glandium)

Tracking

(Blocks 1 bug)

4.9.1
Other
Android
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

7 years ago
Today, I have ordered Ainovo NOVO7 that uses XBusrt 1GHZ (mips).

Let's add Android/mips target since MIPS releases Android NDK for mips chip.
Reporter

Comment 1

7 years ago
Posted patch fix (obsolete) — Splinter Review
Reporter

Updated

7 years ago
Attachment #603966 - Flags: review?(ted.mielczarek)
Reporter

Updated

7 years ago
Blocks: android-mips
Comment on attachment 603966 [details] [diff] [review]
fix

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

::: pr/include/pratom.h
@@ +140,2 @@
>             (defined(__powerpc__) && !defined(__powerpc64__)) || \
> +           ((defined(__arm__) || defined(__i386__) || defined(__mips__)) && \

I'm having a real hard time following what you've done here. It doesn't help that this conditional was a huge mess in the first place. Can you explain this?

::: pr/include/md/_linux.h
@@ +224,5 @@
> +
> +#define _MD_ATOMIC_INCREMENT(ptr) __sync_add_and_fetch(ptr, 1)
> +#define _MD_ATOMIC_DECREMENT(ptr) __sync_sub_and_fetch(ptr, 1)
> +#define _MD_ATOMIC_SET(ptr, nv) __sync_lock_test_and_set(ptr, nv)
> +#define _MD_ATOMIC_ADD(ptr, i) __sync_add_and_fetch(ptr, i)

I don't know the first thing about MIPS, so I'm assuming you know this works properly.
Comment on attachment 603966 [details] [diff] [review]
fix

Clearing review until my questions are answered.
Attachment #603966 - Flags: review?(ted.mielczarek)
Reporter

Comment 4

7 years ago
Android NDK r8 supports MIPS, but mips toolchain name is different of old toolchanin by MIPS.  So I need update patch.

(In reply to Ted Mielczarek [:ted] from comment #2)
> Comment on attachment 603966 [details] [diff] [review]
> fix
> 
> Review of attachment 603966 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: pr/include/pratom.h
> @@ +140,2 @@
> >             (defined(__powerpc__) && !defined(__powerpc64__)) || \
> > +           ((defined(__arm__) || defined(__i386__) || defined(__mips__)) && \
> 
> I'm having a real hard time following what you've done here. It doesn't help
> that this conditional was a huge mess in the first place. Can you explain
> this?

This supports is for atomic built-in function by GCC.
 
> ::: pr/include/md/_linux.h
> @@ +224,5 @@
> > +
> > +#define _MD_ATOMIC_INCREMENT(ptr) __sync_add_and_fetch(ptr, 1)
> > +#define _MD_ATOMIC_DECREMENT(ptr) __sync_sub_and_fetch(ptr, 1)
> > +#define _MD_ATOMIC_SET(ptr, nv) __sync_lock_test_and_set(ptr, nv)
> > +#define _MD_ATOMIC_ADD(ptr, i) __sync_add_and_fetch(ptr, i)
> 
> I don't know the first thing about MIPS, so I'm assuming you know this works
> properly.

Yes.  this is for using gcc built-in function.  If __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4, gcc supports __sync_* for intptr on 32-bit OS.

But I should split a bug for gcc atomic support on various CPU.
Assignee

Comment 5

7 years ago
This is enough to build nspr with NDK r8, targetting mipsel-linux-android. I'll let Kato-san file a separate bug for atomic support.
Attachment #625363 - Flags: review?(ted.mielczarek)
Assignee

Comment 6

7 years ago
The patch depends on the patch from bug 756575.
Depends on: 756575
Assignee

Updated

7 years ago
Assignee: m_kato → mh+mozilla
Assignee

Comment 7

7 years ago
Attachment #625367 - Flags: review?(ted.mielczarek)
Assignee

Comment 8

7 years ago
Comment on attachment 625367 [details]
Allow to build for mipsel with Android NDK r8 (libffi part)

Wrong bug for this patch.
Attachment #625367 - Attachment is obsolete: true
Attachment #625367 - Attachment is patch: false
Attachment #625367 - Flags: review?(ted.mielczarek)
Assignee

Updated

7 years ago
Attachment #603966 - Attachment is obsolete: true
Assignee

Updated

7 years ago
No longer blocks: android-mips
Assignee

Updated

7 years ago
Blocks: android-mips
Assignee

Updated

7 years ago
No longer depends on: 756575
Assignee

Updated

7 years ago
Depends on: 756575
Attachment #625363 - Flags: review?(ted.mielczarek) → review+
Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.330; previous revision: 1.329
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.333; previous revision: 1.332
done
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Version: other → 4.9.1
You need to log in before you can comment on or make changes to this bug.