The default bug view has changed. See this FAQ.

arm detection should have correct define when -mfpu=neon on gcc or on MSVC

RESOLVED FIXED in mozilla16

Status

()

Core
XPCOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: m_kato, Unassigned)

Tracking

Trunk
mozilla16
ARM
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
- 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.
(Reporter)

Comment 1

5 years ago
Created attachment 631805 [details] [diff] [review]
fix
Attachment #631805 - Flags: review?(tterribe)
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".
Attachment #631805 - Flags: review?(tterribe) → review+
(Reporter)

Comment 3

5 years ago
(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.
(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.
(Reporter)

Comment 5

5 years ago
(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...
(Reporter)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/63a91a66c14c
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/63a91a66c14c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.