Last Comment Bug 763378 - arm detection should have correct define when -mfpu=neon on gcc or on MSVC
: arm detection should have correct define when -mfpu=neon on gcc or on MSVC
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: ARM All
: -- normal (vote)
: mozilla16
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-10 23:03 PDT by Makoto Kato [:m_kato]
Modified: 2012-06-19 01:17 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (1.31 KB, patch)
2012-06-10 23:04 PDT, Makoto Kato [:m_kato]
tterribe: review+
Details | Diff | Splinter Review

Description Makoto Kato [:m_kato] 2012-06-10 23:03:13 PDT
- When using -mfpu=neon, we should define MOZILLA_PRESUME_NEON=1
- On MSVC, _M_ARM is cpu architecture.  We should not be always 3 for MOZILLA_ARM_ARCH on CPU.
Comment 1 Makoto Kato [:m_kato] 2012-06-10 23:04:22 PDT
Created attachment 631805 [details] [diff] [review]
fix
Comment 2 Timothy B. Terriberry (:derf) 2012-06-11 00:12:43 PDT
Comment on attachment 631805 [details] [diff] [review]
fix

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

(In reply to Makoto Kato from comment #0)
> - When using -mfpu=neon, we should define MOZILLA_PRESUME_NEON=1

Should we also #define MOZILLA_PRESUME_ARMV6 and MOZILLA_PRESUME_EDSP in this case?

::: xpcom/glue/arm.h
@@ +72,5 @@
>  #      define MOZILLA_MAY_SUPPORT_NEON 1
>  #    endif
>  #  endif
>  
> +  // when using -mfpu=neon, gcc generates neon instruction.

Capital letter on "when", please. Also "instruction" should be "instructions".
Comment 3 Makoto Kato [:m_kato] 2012-06-14 01:14:03 PDT
(In reply to Timothy B. Terriberry (:derf) from comment #2)
> Comment on attachment 631805 [details] [diff] [review]
> fix
> 
> Review of attachment 631805 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> (In reply to Makoto Kato from comment #0)
> > - When using -mfpu=neon, we should define MOZILLA_PRESUME_NEON=1
> 
> Should we also #define MOZILLA_PRESUME_ARMV6 and MOZILLA_PRESUME_EDSP in
> this case?

new ARMv8 supports NEON, but architecture isn't compatible with ARMv6.  So I don't think so.
Comment 4 Timothy B. Terriberry (:derf) 2012-06-14 02:08:02 PDT
(In reply to Makoto Kato from comment #3)
> new ARMv8 supports NEON, but architecture isn't compatible with ARMv6.  So I
> don't think so.

Hmm? I thought AArch32 was fully backwards-compatible with ARMv7, including the ARMv6 "media" instructions (which is all MOZILLA_PRESUME_ARMV6 implies). But I suppose we can defer this until there are actual ARMv8 toolchains and chips.
Comment 5 Makoto Kato [:m_kato] 2012-06-14 03:35:28 PDT
(In reply to Timothy B. Terriberry (:derf) from comment #4)
> (In reply to Makoto Kato from comment #3)
> > new ARMv8 supports NEON, but architecture isn't compatible with ARMv6.  So I
> > don't think so.
> 
> Hmm? I thought AArch32 was fully backwards-compatible with ARMv7, including
> the ARMv6 "media" instructions (which is all MOZILLA_PRESUME_ARMV6 implies).
> But I suppose we can defer this until there are actual ARMv8 toolchains and
> chips.

If 64bit arm, no AArch32, __ARM_NEON__ may be defined.  But, of course, ARMv8 gcc/binutils doesn't release yet...
Comment 7 Ed Morley [:emorley] 2012-06-19 01:17:59 PDT
https://hg.mozilla.org/mozilla-central/rev/63a91a66c14c

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