Last Comment Bug 654873 - ARM target (gcc 4.5.2 or later) should use built-in atomic function instead of arm-kuser
: ARM target (gcc 4.5.2 or later) should use built-in atomic function instead o...
Status: RESOLVED FIXED
[fixed-in-tracemonkey]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: ARM Linux
: -- normal (vote)
: ---
Assigned To: Makoto Kato [:m_kato]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-04 15:57 PDT by Makoto Kato [:m_kato]
Modified: 2011-05-10 15:12 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (1.25 KB, patch)
2011-05-06 00:13 PDT, Makoto Kato [:m_kato]
gal: review+
Details | Diff | Review

Description Makoto Kato [:m_kato] 2011-05-04 15:57:51 PDT
Now, jslock uses arm-kuser that is the atomic functions into Linux kernel.  But gcc 4.5.2 or later has __sync_bool_compare_and_swap().  We should use it.


current code,
00000000 <_Z17js_CompareAndSwapPViii>:
   0:   b5f8            push    {r3, r4, r5, r6, r7, lr}
   2:   f44f 667c       mov.w   r6, #4032       ; 0xfc0
   6:   4605            mov     r5, r0
   8:   460c            mov     r4, r1
   a:   4617            mov     r7, r2
   c:   f6cf 76ff       movt    r6, #65535      ; 0xffff
  10:   4620            mov     r0, r4
  12:   4639            mov     r1, r7
  14:   462a            mov     r2, r5
  16:   47b0            blx     r6    <---- Call kernel code
  18:   b950            cbnz    r0, 30 <_Z17js_CompareAndSwapPViii+0x30>
  1a:   f1d0 0001       rsbs    r0, r0, #1
  1e:   bf38            it      cc
  20:   2000            movcc   r0, #0
  22:   bdf8            pop     {r3, r4, r5, r6, r7, pc}
  24:   f3af 8000       nop.w
  28:   f3af 8000       nop.w
  2c:   f3af 8000       nop.w
  30:   682b            ldr     r3, [r5, #0]
  32:   429c            cmp     r4, r3
  34:   d0ec            beq.n   10 <_Z17js_CompareAndSwapPViii+0x10>
  36:   f1d0 0001       rsbs    r0, r0, #1
  3a:   bf38            it      cc
  3c:   2000            movcc   r0, #0
  3e:   bdf8            pop     {r3, r4, r5, r6, r7, pc}

after (using __sync_bool_compare_and_swap)
00000000 <_Z17js_CompareAndSwapPViii>:
   0:   f3bf 8f5f       dmb     sy
   4:   e850 3f00       ldrex   r3, [r0]
   8:   428b            cmp     r3, r1
   a:   d107            bne.n   1c <_Z17js_CompareAndSwapPViii+0x1c>
   c:   e840 2300       strex   r3, r2, [r0]
  10:   f093 0f00       teq     r3, #0
  14:   d1f6            bne.n   4 <_Z17js_CompareAndSwapPViii+0x4>
  16:   460b            mov     r3, r1
  18:   f3bf 8f5f       dmb     sy
  1c:   1a5b            subs    r3, r3, r1
  1e:   4258            negs    r0, r3
  20:   eb40 0003       adc.w   r0, r0, r3
  24:   4770            bx      lr
  26:   bf00            nop
  28:   f3af 8000       nop.w
  2c:   f3af 8000       nop.w
Comment 1 Andreas Gal :gal 2011-05-04 16:01:39 PDT
Nice.
Comment 2 Makoto Kato [:m_kato] 2011-05-06 00:13:19 PDT
Created attachment 530557 [details] [diff] [review]
fix
Comment 3 Andreas Gal :gal 2011-05-06 00:20:51 PDT
Comment on attachment 530557 [details] [diff] [review]
fix

Cool. Thanks. I assume this is guaranteed to work with all ARM architectures we support?
Comment 4 Makoto Kato [:m_kato] 2011-05-06 00:39:37 PDT
(In reply to comment #3)
> Comment on attachment 530557 [details] [diff] [review] [review]
> fix
> 
> Cool. Thanks. I assume this is guaranteed to work with all ARM architectures
> we support?

Our target platform is ARMv7 architecture (Android, Maemo and MeeGo).

__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 is only defined when -march or -mcpu is such as ARMv7 that has barrier instructions (dmb and strex).

So if using -march=armv4t (ARMv4 Thumb), __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 isn't defined.
Comment 5 Makoto Kato [:m_kato] 2011-05-07 08:52:12 PDT
http://hg.mozilla.org/tracemonkey/rev/9ec7a17e3136
Comment 6 Chris Leary [:cdleary] (not checking bugmail) 2011-05-10 15:12:19 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/9ec7a17e3136

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