Runtime NEON detection is deprecated

[Egor Pasko]

Chrome for Android/ARM enables NEON instructions at compile time, as of M50 (except in net/, see below). I think it is a good time to start removing NEON *runtime* detection. Here is a list of places I found with runtime detection in use:

webrtc (giving it a try: https://codereview.webrtc.org/1955413003/)

libjpeg_turbo

openmax_dl

pdfium

skia

webp

If you like removing code like I do, please feel free to pick your victim :) Once all these are cleaned up, we can remove the toplevel GN/GYP variable.

The below:

Cronet supports older devices than Chrome does, and that includes pre-NEON.

Also, more interestingly, there was one hiccup during rollout, where we enabled NEON in boringssl even for devices that implement NEON support in a subtly broken way. This issue with "broken NEON" is uncommon and seems *not* to affect the code emitted by the compiler, or any other-than-boringssl hand-written assembly, but it is certainly something to watch out in the future. So, boringssl will have its own NEON detection, including its own auxval/cpuinfo hacks. See details in http://crbug.com/606629

Future: remove pre-NEON code enabled at compile time. Things to watch out for: we cannot remove anything that Cast Audio depends on. Cast folks: do you have a list? I guess there is no need in non-neon fallbacks for: ffmpeg, skia, webp, libjpeg, pdfium. Not sure about webrtc and maybe I missed something that can be cleaned up.

[David Benjamin]

On Tue, May 10, 2016 at 6:56 AM Egor Pasko <pa...@chromium.org> wrote:

Chrome for Android/ARM enables NEON instructions at compile time, as of M50 (except in net/, see below). I think it is a good time to start removing NEON *runtime* detection. Here is a list of places I found with runtime detection in use:

webrtc (giving it a try: https://codereview.webrtc.org/1955413003/)

libjpeg_turbo

openmax_dl

pdfium

skia

webp

If you like removing code like I do, please feel free to pick your victim :) Once all these are cleaned up, we can remove the toplevel GN/GYP variable.

The below:

Cronet supports older devices than Chrome does, and that includes pre-NEON.

Also, more interestingly, there was one hiccup during rollout, where we enabled NEON in boringssl even for devices that implement NEON support in a subtly broken way. This issue with "broken NEON" is uncommon and seems *not* to affect the code emitted by the compiler, or any other-than-boringssl hand-written assembly, but it is certainly something to watch out in the future. So, boringssl will have its own NEON detection, including its own auxval/cpuinfo hacks. See details in http://crbug.com/606629

(Even independent of this, BoringSSL will need to runtime-detect NEON features until all of our consumers are done supporting older devices.)

Once it makes its way to stable, the Net.HasBrokenNEON metric will measure how common this chip is. I doubt this will be the last we've seen of that CPU bug. :-/

[Mike Frysinger]

just to be clear, you're proposing removing runtime detection only, not support for non-neon devices ?  i.e. you're not proposing making neon a hard requirement ?

for Chromium, i can understand making this a requirement, and that's fine.  but forcing the changes into downstream projects seems inappropriate.

-mike

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Wei Zhong]

For Cast Audio, we don't use runtime NEON detection.

In stead, we define "arm_use_neon=false" specifically for audio build. We are OK with the proposal as long as the VFP or C implementation is still available at compile time.

Thanks.

---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

[Egor Pasko]

On Tue, May 10, 2016 at 4:21 PM, Mike Frysinger <vap...@chromium.org> wrote:

just to be clear, you're proposing removing runtime detection only, not support for non-neon devices ?  i.e. you're not proposing making neon a hard requirement ?

In this iteration of cleanup the proposal is to only remove the runtime detection. Fallbacks to old VFP can be enabled at compile time.

for Chromium, i can understand making this a requirement, and that's fine.  but forcing the changes into downstream projects seems inappropriate.

I am slightly confused.

Did you mean that removing the compile-time switch causes problems for downstream projects like Cast/Opera? Or is it also about runtime detection removal that is inappropriate?

Or maybe you mean upstream projects like webrtc/skia?

[Egor Pasko]

On Tue, May 10, 2016 at 4:35 PM, Wei Zhong <wzh...@google.com> wrote:

For Cast Audio, we don't use runtime NEON detection.

In stead, we define "arm_use_neon=false" specifically for audio build. We are OK with the proposal as long as the VFP or C implementation is still available at compile time.

Right, as agreed before arm_use_neon=false will continue to be supported for Cast Audio targets. In the "Future" part of my original email I ask whether we can later clean up some stuff under arm_use_neon=false that is *not* used in Cast.

[Mike Frysinger]

On Tue, May 10, 2016 at 12:16 PM, Egor Pasko <pa...@chromium.org> wrote:

On Tue, May 10, 2016 at 4:21 PM, Mike Frysinger <vap...@chromium.org> wrote:

just to be clear, you're proposing removing runtime detection only, not support for non-neon devices ?  i.e. you're not proposing making neon a hard requirement ?

In this iteration of cleanup the proposal is to only remove the runtime detection. Fallbacks to old VFP can be enabled at compile time.

for Chromium, i can understand making this a requirement, and that's fine.  but forcing the changes into downstream projects seems inappropriate.

I am slightly confused.

Did you mean that removing the compile-time switch causes problems for downstream projects like Cast/Opera? Or is it also about runtime detection removal that is inappropriate?

Or maybe you mean upstream projects like webrtc/skia?

i'm thinking of projects like pdfium that, while hosted under the Chromium umbrella, are a standalone project by themselves.  i don't think Chromium's (the browser) goals should be dictating that pdfium drop support for non-neon systems.  or for projects that aren't even under Chromium's umbrella like libjpeg-turbo.

in general, if maintaining runtime detection is onerous, i don't have a problem dropping it.  it's when you say "this project requires neon" that i'm disturbed.  i.e. dropping support for neon at compile time.

-mike

[Ilja Friedel]

On Tue, May 10, 2016 at 3:55 AM, Egor Pasko <pa...@chromium.org> wrote:

Chrome for Android/ARM enables NEON instructions at compile time, as of M50 (except in net/, see below). I think it is a good time to start removing NEON *runtime* detection. Here is a list of places I found with runtime detection in use:

webrtc (giving it a try: https://codereview.webrtc.org/1955413003/)

libjpeg_turbo

openmax_dl

pdfium

skia

webp

If you like removing code like I do, please feel free to pick your victim :) Once all these are cleaned up, we can remove the toplevel GN/GYP variable.

The below:

Cronet supports older devices than Chrome does, and that includes pre-NEON.

This was b/27619303.

 

Also, more interestingly, there was one hiccup during rollout, where we enabled NEON in boringssl even for devices that implement NEON support in a subtly broken way. This issue with "broken NEON" is uncommon and seems *not* to affect the code emitted by the compiler, or any other-than-boringssl hand-written assembly, but it is certainly something to watch out in the future. So, boringssl will have its own NEON detection, including its own auxval/cpuinfo hacks. See details in http://crbug.com/606629

Future: remove pre-NEON code enabled at compile time. Things to watch out for: we cannot remove anything that Cast Audio depends on. Cast folks: do you have a list? I guess there is no need in non-neon fallbacks for: ffmpeg, skia, webp, libjpeg, pdfium. Not sure about webrtc and maybe I missed something that can be cleaned up.

[Egor Pasko]

On Tue, May 10, 2016 at 6:22 PM, Mike Frysinger <vap...@chromium.org> wrote:

On Tue, May 10, 2016 at 12:16 PM, Egor Pasko <pa...@chromium.org> wrote:

On Tue, May 10, 2016 at 4:21 PM, Mike Frysinger <vap...@chromium.org> wrote:

just to be clear, you're proposing removing runtime detection only, not support for non-neon devices ?  i.e. you're not proposing making neon a hard requirement ?

In this iteration of cleanup the proposal is to only remove the runtime detection. Fallbacks to old VFP can be enabled at compile time.

for Chromium, i can understand making this a requirement, and that's fine.  but forcing the changes into downstream projects seems inappropriate.

I am slightly confused.

Did you mean that removing the compile-time switch causes problems for downstream projects like Cast/Opera? Or is it also about runtime detection removal that is inappropriate?

Or maybe you mean upstream projects like webrtc/skia?

i'm thinking of projects like pdfium that, while hosted under the Chromium umbrella, are a standalone project by themselves.  i don't think Chromium's (the browser) goals should be dictating that pdfium drop support for non-neon systems.  or for projects that aren't even under Chromium's umbrella like libjpeg-turbo.

in general, if maintaining runtime detection is onerous, i don't have a problem dropping it.  it's when you say "this project requires neon" that i'm disturbed.  i.e. dropping support for neon at compile time.

Ah. Understood.

Certainly the choice whether to do the cleaning is up to the individual external projects. I know some projects wanted to remove this cruft (Skia!), so now they are very welcome to do so from the Chromium standpoint. Even further, since Cast is not using Skia with non-Neon tartets, it feels safe (for Chrome) to remove the non-neon compile-time support as well. Again, only if all consumers of Skia are OK with the plan.

We'd still like to ask to remove the runtime detection support from GYP/GN (so that we can remove it from build/common.gypi), other build systems may still support it if needed.

 

On Tue, May 10, 2016 at 6:55 AM, Egor Pasko <pa...@chromium.org> wrote:

Chrome for Android/ARM enables NEON instructions at compile time, as of M50 (except in net/, see below). I think it is a good time to start removing NEON *runtime* detection. Here is a list of places I found with runtime detection in use:

webrtc (giving it a try: https://codereview.webrtc.org/1955413003/)

libjpeg_turbo

openmax_dl

pdfium

skia

webp

If you like removing code like I do, please feel free to pick your victim :) Once all these are cleaned up, we can remove the toplevel GN/GYP variable.

The below:

Cronet supports older devices than Chrome does, and that includes pre-NEON.

Also, more interestingly, there was one hiccup during rollout, where we enabled NEON in boringssl even for devices that implement NEON support in a subtly broken way. This issue with "broken NEON" is uncommon and seems *not* to affect the code emitted by the compiler, or any other-than-boringssl hand-written assembly, but it is certainly something to watch out in the future. So, boringssl will have its own NEON detection, including its own auxval/cpuinfo hacks. See details in http://crbug.com/606629

Future: remove pre-NEON code enabled at compile time. Things to watch out for: we cannot remove anything that Cast Audio depends on. Cast folks: do you have a list? I guess there is no need in non-neon fallbacks for: ffmpeg, skia, webp, libjpeg, pdfium. Not sure about webrtc and maybe I missed something that can be cleaned up.

--

Egor Pasko

[Fredrik Öhrn]

Hi!

On Tuesday, May 10, 2016 at 6:42:00 PM UTC+2, pasko wrote:

On Tue, May 10, 2016 at 6:22 PM, Mike Frysinger <vap...@chromium.org> wrote:

On Tue, May 10, 2016 at 12:16 PM, Egor Pasko <pa...@chromium.org> wrote:

On Tue, May 10, 2016 at 4:21 PM, Mike Frysinger <vap...@chromium.org> wrote:

just to be clear, you're proposing removing runtime detection only, not support for non-neon devices ?  i.e. you're not proposing making neon a hard requirement ?

In this iteration of cleanup the proposal is to only remove the runtime detection. Fallbacks to old VFP can be enabled at compile time.

for Chromium, i can understand making this a requirement, and that's fine.  but forcing the changes into downstream projects seems inappropriate.

I am slightly confused.

Did you mean that removing the compile-time switch causes problems for downstream projects like Cast/Opera? Or is it also about runtime detection removal that is inappropriate?

Or maybe you mean upstream projects like webrtc/skia?

i'm thinking of projects like pdfium that, while hosted under the Chromium umbrella, are a standalone project by themselves.  i don't think Chromium's (the browser) goals should be dictating that pdfium drop support for non-neon systems.  or for projects that aren't even under Chromium's umbrella like libjpeg-turbo.

in general, if maintaining runtime detection is onerous, i don't have a problem dropping it.  it's when you say "this project requires neon" that i'm disturbed.  i.e. dropping support for neon at compile time.

Ah. Understood.

Certainly the choice whether to do the cleaning is up to the individual external projects. I know some projects wanted to remove this cruft (Skia!), so now they are very welcome to do so from the Chromium standpoint. Even further, since Cast is not using Skia with non-Neon tartets, it feels safe (for Chrome) to remove the non-neon compile-time support as well. Again, only if all consumers of Skia are OK with the plan.

We'd still like to ask to remove the runtime detection support from GYP/GN (so that we can remove it from build/common.gypi), other build systems may still support it if needed.

 

We've collected some stats and Opera for Android has on the order of 1000 weekly users that use devices running ARMv7 without NEON (mostly old Tegra chipsets).

Since the Play Store is incapable of differentiating between NEON and non-NEON devices these uses will be left out in the cold. Extra sad is the fact that whenever the future NEON-only upgrade arrives it'll just brick the browser on these phones without recourse.

How is Chrome planning to handle this?

/Fredrik

[Raymond Toy]

Does this mean I can now also add NEON instrinsics into the code?  Of course, the non-neon code needs to stay around, but it would be nice to be able to have this now in some WebAudio code to optimize some heavy code paths.   We do this today already with x86 and SSE2 intrinsics.  Doing this for Arm would be really nice.

[Egor Pasko]

We blocked a few most popular Tegra 2 devices in Play by name, so they stop receiving Chrome updates. Some ROMs running Tegra2 keep saying that they are non-Tegra2, we cannot support these.

--

Egor Pasko

[Egor Pasko]

On Wed, May 11, 2016 at 9:52 PM, Raymond Toy <rt...@google.com> wrote:

Does this mean I can now also add NEON instrinsics into the code?  Of course, the non-neon code needs to stay around, but it would be nice to be able to have this now in some WebAudio code to optimize some heavy code paths.   We do this today already with x86 and SSE2 intrinsics.  Doing this for Arm would be really nice.

I think it was allowed before? If the code is guarded by arm_use_neon=true, you can apply neon-specific intrinsics/assembly/optimizations at your will. I would personally prefer intrinsics, but that's another story.

"Just" need to make sure that the code you add intrinsics to indeed runs only on neon:

1. Cast Audio continues to be supported with the compile-time switch

2. you are not breaking your other, non-chromium downstreams

[Jochen Eisinger]

+u...@chromium.org 

Note that V8 uses runtime detection if arm_use_neon is false, and otherwise just uses neon if arm_use_neon is true

[Fredrik Öhrn]

Thanks for the information. We'll do the same.


/Fredrik

--
"We do what we must because we can." -Aperture Science

Fredrik Öhrn Opera Software
ohrn @ opera . com Sweden

[Raymond Toy]

On Thu, May 12, 2016 at 4:24 AM, Egor Pasko <pa...@chromium.org> wrote:

On Wed, May 11, 2016 at 9:52 PM, Raymond Toy <rt...@google.com> wrote:

Does this mean I can now also add NEON instrinsics into the code?  Of course, the non-neon code needs to stay around, but it would be nice to be able to have this now in some WebAudio code to optimize some heavy code paths.   We do this today already with x86 and SSE2 intrinsics.  Doing this for Arm would be really nice.

I think it was allowed before? If the code is guarded by arm_use_neon=true, you can apply neon-specific intrinsics/assembly/optimizations at your will. I would personally prefer intrinsics, but that's another story.

Yes, sort of.  IIRC, chromium on android was always compiled with arm_use_neon=false; I didn't want to add runtime detection and split up the original source files and build two libraries (one for neon and one without).  (I did do this for openmax_dl since it's a huge win to be able to use neon for FFTs.)

But now since detection isn't required I can just use the appropriate compile time flag to enable/disable neon.  Thus, no runtime detection and no need to split up the file into two pieces.

So, I take this to mean I can now use neon to my hearts content with arm_use_neon=true, so long as I make sure that the code still works if arm_use_neon=false.

"Just" need to make sure that the code you add intrinsics to indeed runs only on neon:

1. Cast Audio continues to be supported with the compile-time switch

2. you are not breaking your other, non-chromium downstreams

Yeah, the code in question is only in blink, so 2) isn't a problem.  1) is solved by keeping the original non-neon code around.

[Egor Pasko]

On Thu, May 12, 2016 at 1:28 PM, Jochen Eisinger <joc...@chromium.org> wrote:

+u...@chromium.org 

Note that V8 uses runtime detection if arm_use_neon is false, and otherwise just uses neon if arm_use_neon is true

I cannot read these parts of v8 code reliably. It seems that detection is performed in the constructor of class CPU:

https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/base/cpu.cc&sq=package:chromium&type=cs&l=509

And then the only place where it is used:

https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/arm/assembler-arm.cc&sq=package:chromium&type=cs&l=120

To me this reads as:

1. v8 always performs the NEON check

2. with arm_use_neon==true the result of the check is used, otherwise it is ignored

Looks contrary to what you said .. so probably I am holding it wrong :/

[Egor Pasko]

On Thu, May 12, 2016 at 7:31 PM, Raymond Toy <rt...@google.com> wrote:

On Thu, May 12, 2016 at 4:24 AM, Egor Pasko <pa...@chromium.org> wrote:

On Wed, May 11, 2016 at 9:52 PM, Raymond Toy <rt...@google.com> wrote:

Does this mean I can now also add NEON instrinsics into the code?  Of course, the non-neon code needs to stay around, but it would be nice to be able to have this now in some WebAudio code to optimize some heavy code paths.   We do this today already with x86 and SSE2 intrinsics.  Doing this for Arm would be really nice.

I think it was allowed before? If the code is guarded by arm_use_neon=true, you can apply neon-specific intrinsics/assembly/optimizations at your will. I would personally prefer intrinsics, but that's another story.

Yes, sort of.  IIRC, chromium on android was always compiled with arm_use_neon=false;

Ah, right, it has been the GYP version of it "arm_neon=0 arm_neon_optional=1". So to ship the intrinsics with Chrome for Android this should have been under arm_neon_optional==1. Understood.

I didn't want to add runtime detection and split up the original source files and build two libraries (one for neon and one without).  (I did do this for openmax_dl since it's a huge win to be able to use neon for FFTs.)

But now since detection isn't required I can just use the appropriate compile time flag to enable/disable neon.  Thus, no runtime detection and no need to split up the file into two pieces.

So, I take this to mean I can now use neon to my hearts content with arm_use_neon=true, so long as I make sure that the code still works if arm_use_neon=false.

 

Yes, please please please!

[Ulan Degenbaev]

There is a line before:

https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/arm/assembler-arm.cc&l=84

supported_ |= CpuFeaturesImpliedByCompiler()

This line uses the compile time flag. Once the neon bit is set in supported_, the later runtime detection cannot clear it.

Cheers,

Ulan.

[Egor Pasko]

On Tue, May 17, 2016 at 5:04 PM, Ulan Degenbaev <ul...@google.com> wrote:

There is a line before:

https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/arm/assembler-arm.cc&l=84

supported_ |= CpuFeaturesImpliedByCompiler()

This line uses the compile time flag. Once the neon bit is set in supported_, the later runtime detection cannot clear it.

Ah, thanks!

With Chromium knowing about NEON at compile time, does v8 have remaining clients that need runtime detection?

--

Egor Pasko