Repository: zeux/meshoptimizer (language: cpp)

Issue:
Builtins not there in Android NDK 19c ("Clang 8.0.2")
I don't know if this is worth a fix, but documenting it for posterity at least.

Similar to #130/Apple, Android NDK 19c/"Clang 8.0.2" passes this check in vertexfilter:

https://github.com/zeux/meshoptimizer/blob/29a6b8ccb9891c4c14964f9dc164602cebc9c864/src/vertexfilter.cpp#L188-L194

But Android builds targeting ARM v8 ABIs with this NDK fail because this logic allows its special rev of Clang 8.0.2, which doesn't seem to have the builtin, through:

```
.../meshoptimizer-src/src/vertexfilter.cpp:189:9: error: use of undeclared identifier '__builtin_rotateleft64'
        return __builtin_rotateleft64(v, x);
               ^
1 error generated.
```

The easy workaround is to upgrade the NDK to a better-supported version (NDK 23c/Clang r416183c2/LLVM 12-ish builds without dropping Jelly Bean support).

### Too much detail

NDK 19c's specific rev of Clang 8:

```shell
$ android/sdk/ndk/19.2.5345600/toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ --version
Android (5058415 based on r339409) clang version 8.0.2
(https://android.googlesource.com/toolchain/clang 40173bab62ec746213857d083c0e8b0abb568790)
(https://android.googlesource.com/toolchain/llvm 7a6618d69e7e8111e1d49dc9e7813767c5ca756a)
(based on LLVM 8.0.2svn)
```

The `rotateleft64` builtin doesn't appear to have been added until closer to LLVM 9 (https://github.com/llvm/llvm-project/commit/ad82390d3f338a4df8afb8df632b722790d283ba, https://github.com/llvm/llvm-project/blob/905af40cc6ce0d246e42bf7db456d8ca944caaf6/llvm/CMakeLists.txt) even though it's in tags as far back as 8.0.0.

It's just not there in this "8.0.2":

```shell
# NDK 23c:
$ grep __builtin_rotate ./23.2.8568313/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/12.0.9/include/ia32intrin.h
  return __builtin_rotateleft8(__X, __C);
  return __builtin_rotateright8(__X, __C);
  return __builtin_rotateleft16(__X, __C);
  return __builtin_rotateright16(__X, __C);
  return __builtin_rotateleft32(__X, __C);
  return __builtin_rotateright32(__X, __C);
  return __builtin_rotateleft64(__X, __C);
  return __builtin_rotateright64(__X, __C);

# NDK 19c:
$ grep -R __builtin_rotate ./19.2.5345600/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/8.0.2/
$ echo $?
1
```


Discussion / hints:
I’m assuming it would be sufficient to bump the cutoff to 9? It’s not particularly critical to use this builtin on dated compilers anyway.
Maybe? Or it might break someone else's edge case. The discussion around #130 suggested using `__has_builtin`  to make the check based on the functionality instead of Clang's reported major version.

Or, this could simply document that if the check fails, the toolchain is nonstandard and possibly outdated.
(Want to be clear that my feelings wouldn't be hurt in the slightest if this issue was closed. I just wanted to make the reason more visible to anyone else who stumbles on it.)

Fix patch:
diff --git a/src/vertexfilter.cpp b/src/vertexfilter.cpp
index 14a73b1dd..4fc118648 100644
--- a/src/vertexfilter.cpp
+++ b/src/vertexfilter.cpp
@@ -63,6 +63,10 @@
 #define wasmx_unziphi_v32x4(a, b) wasm_v32x4_shuffle(a, b, 1, 3, 5, 7)
 #endif
 
+#ifndef __has_builtin
+#define __has_builtin(x) 0
+#endif
+
 namespace meshopt
 {
 
@@ -185,9 +189,7 @@ inline uint64_t rotateleft64(uint64_t v, int x)
 {
 #if defined(_MSC_VER) && !defined(__clang__)
 	return _rotl64(v, x);
-// Apple's Clang 8 is actually vanilla Clang 3.9, there we need to look for
-// version 11 instead: https://en.wikipedia.org/wiki/Xcode#Toolchain_versions
-#elif defined(__clang__) && ((!defined(__apple_build_version__) && __clang_major__ >= 8) || __clang_major__ >= 11)
+#elif defined(__clang__) && __has_builtin(__builtin_rotateleft64)
 	return __builtin_rotateleft64(v, x);
 #else
 	return (v << (x & 63)) | (v >> ((64 - x) & 63));


Test patch: