https://cs.chromium.org/search/?q=ARM_NEON_INTRINSICS&sq=package:chromium&type=cs

Looks like this impacts webgl and audio? Why does clang not pretend to have at least gcc 4.7 support?  Can we fix the version macro? Does base or skia have a macro for this?

I believe that is the affected code, yes.  That but also subsequent patches trying to be consistent with tree-specific idioms.

It looks like Clang started at GCC 4.2.1 back when it made sense, but never changed subsequently.
http://lists.llvm.org/pipermail/cfe-dev/2012-May/thread.html#21389

For NEON detection, src/media/base uses `defined(ARCH_CPU_ARM_FAMILY) && defined(USE_NEON)` (src/base doesn't mention NEON), and skia defines `defined(SK_ARM_HAS_NEON)` occasionally combined with `defined(SK_CPU_LENDIAN)` (this gets defined in a mix of header and makefile, so it's not completely clear to me that it would be reliably defined for an out-of-tree reference).

For GCC version, I can't see any such tests in skia, while base appears to refer to the three GCC version macros directly (doing it the hard way) with further tests for clang alongside that.  At least as far as I've looked Clang is considered separately where its GCC version number would fail, but I haven't looked at everything.  Most references to __GNUC_MINOR__ occur in src/third_party.


Is there any policy/precedent showing that GCC 4.6 is too old to worry about?  If so, maybe the version test could simply be deleted.

thakis@ and dpranke@ know what's up with the build chain.

clang intentionally claims that it's gcc when used in gcc driver mode (e.g. on non-windows) because it tries to be gcc-compatible and more code Just Works that way than if it didn't do that. It intentionally reports that it's 4.2.1, see my link in http://stackoverflow.com/questions/12893731/why-does-clang-dumpversion-report-4-2-1 for why.

Just stick an `|| __clang` in there and this should be good. (But see bug 124610 for clang vs arm assembly -- most stuff works by now, but not all of it)

Just want to confirm (because I don't know the long-term intention for the affected code); is it preferable to patch the specific error until it's replaced or to make this code right and proper for future generations?

I did put together an experimental patch adding COMPILER(CLANG) to most of the COMPILER(GCC) references (some references are genuinely GCC-specific), and making COMPILER(GCC) fail for Clang.  I also have the simple one line fix.

>  is it preferable to patch the specific error until it's replaced or to make this code right and proper for future generations?

If there's a way to make it work with both gcc and clang that the ARM ARM describes, that's what we should do (e.g. add `.syntax unified` somewhere and modernize some old instructions that gcc understands but clang doesn't). If the ARM ARM describes something that doesn't work in clang yet but should, we should file a bug with the clang folks, and the arm clang folks might fix it. (I don't remember if the blink arm code is in a .S file or if it uses intrinsics.)

> adding COMPILER(CLANG) to most of the COMPILER(GCC) references (some references are genuinely GCC-specific), and making COMPILER(GCC) fail for Clang.

I don't think we want to do this.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a66b7eda01fb58b0393ecd4f8086ebab0f25e5a

commit 6a66b7eda01fb58b0393ecd4f8086ebab0f25e5a
Author: simon.hosie <simon.hosie@arm.com>
Date: Tue Dec 20 22:57:13 2016

Enable ARM NEON in Blink if compiling with Clang

Clang declares itself as GCC 4.2.1, which fails a GCC_VERSION test in wtf/CPU.h
and consequently disables NEON code in Blink.

BUG= 673067 

Change-Id: I1e7a28996ab4d016cbc3039556384f00fb3e7034
Review-Url: https://codereview.chromium.org/2569383002
Cr-Commit-Position: refs/heads/master@{#439903}

[modify] https://crrev.com/6a66b7eda01fb58b0393ecd4f8086ebab0f25e5a/third_party/WebKit/Source/wtf/CPU.h