New issue
Advanced search Search tips
Starred by 4 users
Status: Fixed
Owner: ----
Closed: Nov 2016



Sign in to add a comment
configure detection of neon support can no longer rely on arm_neon.h presence
Reported by nish.ara...@canonical.com, Oct 4 2016 Back to list
We currently in Ubuntu (16.10) are seeing a failure to build the libwebp package on armhf: https://launchpadlibrarian.net/285964171/buildlog_ubuntu-yakkety-armhf.libwebp_0.5.1-2_BUILDING.txt.gz

Debugging this, it seems that with newer GCC's (6.2 is in use in this build), the arm_neon.h header has been 'fixed' to always define intrinsics (suitably guarded by target pragmas), while the old behaviour was to error if the target did not support Neon instructions. Thus, configure-detection using only the arm_neon.h's presence and suitability is no longer sufficient to determine if neon support should be included and/or needs the -mfpu=neon flag.

Indeed, to enable neon support at all requires passing the -mfpu=neon flag, I believe, with GCC 6.2.

I am not 100% on the correct fix, but perhaps the Neon configure.ac code can simply be modified to resemble the SSE support checks?
 
Project Member Comment 1 by jz...@google.com, Oct 5 2016
Thanks for the report. The check was added to allow for runtime cpu-detection in the debian build which at the time didn't have a neon requirement while additionally removing inspecting /proc/cpuinfo unnecessarily where the platform forced neon. As you mention this could probably be refined a bit.
I'll see if I can get something setup to reproduce this locally.

In the meantime the offending commit can be reverted locally to get things building [1]. This will behave as previous releases performance wise if NEON isn't forced at the platform level (which it looks like it isn't).

[1] 74fb56f add runtime NEON detection
Project Member Comment 2 by jz...@google.com, Oct 5 2016
Status: Accepted
I can reproduce this. As mentioned the definition has gone from something like the following in gcc-5:

#ifndef __ARM_NEON__
#error You must enable NEON instructions (e.g. -mfloat-abi=softfp -mfpu=neon) to use arm_neon.h
#else

to:

#ifndef __ARM_FP
#error "NEON intrinsics not available with the soft-float ABI.  Please use -mfloat-abi=softp or -mfloat-abi=hard"
#else

#pragma GCC push_options
#pragma GCC target ("fpu=neon")

so another workaround to keep the runtime cpu-detection might be:

diff --git a/src/dsp/neon.h b/src/dsp/neon.h
index 0a06266..0ac3263 100644
--- a/src/dsp/neon.h
+++ b/src/dsp/neon.h
@@ -16,6 +16,9 @@

 #include "./dsp.h"

+#pragma GCC push_options
+#pragma GCC target ("fpu=neon")
+
 // Right now, some intrinsics functions seem slower, so we disable them
 // everywhere except aarch64 where the inline assembly is incompatible.
 #if defined(__aarch64__)
Project Member Comment 3 by jz...@google.com, Oct 7 2016
Status: Started
I think this update [1] should solve the issue. If you have a chance please give it a try. This behaves as expected for me under debian-jessie (gcc-4.8/9) and debian-stretch (gcc-5/6).

[1] https://chromium-review.googlesource.com/#/c/393360/
Project Member Comment 4 by bugdroid1@chromium.org, Oct 14 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/webm/libwebp/+/0104d730bf2485b58a6b9e9b5de0c0dbe170ebf5

commit 0104d730bf2485b58a6b9e9b5de0c0dbe170ebf5
Author: James Zern <jzern@google.com>
Date: Wed Oct 05 07:03:14 2016

configure: fix NEON flag detection under gcc 6

use a compile check on a separate file to avoid assuming using
arm_neon.h is safe to use without flags when just the file itself is
self-contained with GCC target pragmas.

BUG= webp:313 

Change-Id: I48f92ae3e6e4a9468ea5b937c80a89ee40b2dcfd

[modify] https://crrev.com/0104d730bf2485b58a6b9e9b5de0c0dbe170ebf5/configure.ac

Project Member Comment 5 by jz...@google.com, Nov 24 2016
Status: Fixed
This is fixed for the original report, it will appear in 0.5.2. There may be other cases where we would fail in the rtcd check (clang wanting -march=armv7-a), but they can be dealt with in a followup as the result is no worse than previous releases without the check.
Project Member Comment 6 by bugdroid1@chromium.org, Dec 8 2016
Labels: merge-merged-0.5.2
The following revision refers to this bug:
  https://chromium.googlesource.com/webm/libwebp/+/42ebe3b783f96eb230742ad45771cdf7e32ae0a3

commit 42ebe3b783f96eb230742ad45771cdf7e32ae0a3
Author: James Zern <jzern@google.com>
Date: Wed Oct 05 07:03:14 2016

configure: fix NEON flag detection under gcc 6

use a compile check on a separate file to avoid assuming using
arm_neon.h is safe to use without flags when just the file itself is
self-contained with GCC target pragmas.

BUG= webp:313 

Change-Id: I48f92ae3e6e4a9468ea5b937c80a89ee40b2dcfd
(cherry picked from commit 0104d730bf2485b58a6b9e9b5de0c0dbe170ebf5)

[modify] https://crrev.com/42ebe3b783f96eb230742ad45771cdf7e32ae0a3/configure.ac

Sign in to add a comment