libyuv is confused. It some pretty stupid #ifdefs, making a lot of assumptions it shouldn't do. It hurts our clang build, because it did assumptions that we're clang-cl or clang GCC randomly. Now that we use -fms-extensions, we can actually use any version of assemblies - both MSVC and GCC variants should work, but we need libyuv to be consequent. I hacked it to use MSVC versions consequently. I'm tempted to try using -fms-compatibility and see if we can get it working this way.

libyuv has never seen a mingw-clang build so it is all confused about what assembly implementations it should use. mingw-clang does not define _MSC_VER without -fms-compatibility so everywhere we see _MSC_VER we generally need to add an identical case for clang-on-windows This makes some of the if statements very complicated, so we simplified some of them by turning it into two if statements. I detect AVX2 support with __clang_major__ > 3 which is not strictly accurate: some versions of clang 3 have AVX2 support; but it is fairly close and we only barely support clang 3.9

So I have a patch to libyuv that compiles on all platforms. There's a different way we could (try to) solve this though: we could ask mingw-clang to define _MSC_VER using a build flag. This would probably remove the need for most of the #if changes in the patch; but we would probably still need to make a few. We would also need to add it to the .gyp build files. I'm also unsure what your local-patch-vs-upstream-changes policy for this library is. Randell, could you weigh in on what you'd like to see here?

We could run that sort of patch, though the likelihood of conflicts on updates is significant. if we did, I'd try to get it upstreamed (or a more general fix upstreamed, to factor out all those ifdef changes into one define). We could also set _MSC_VER for now locally, and file an issue/your-patch upstream with the libyuv maintainer so it can get fixed in our next import (or some later one); this side-steps merge issues. Both work; the big deal is not carrying this patch long-term and dealing with hand-fixes on updates

Turns out Martin found and fixed this for us. We just need to take these patches: https://chromium.googlesource.com/libyuv/libyuv/+/d8680893ecfcffd952c7a5d0dc1878d1c486a4e4%5E%21/ https://chromium.googlesource.com/libyuv/libyuv/+/6e498475c749063485a046dfb24e1fa1a81cbedf%5E%21/ I'm going to send in a patch for these, and then when we update we'll automatically get the fixes.

This patch combines the following two upstream patches (which we are missing, because we're out of date): https://chromium.googlesource.com/libyuv/libyuv/+/d8680893ecfcffd952c7a5d0dc1878d1c486a4e4%5E%21/ https://chromium.googlesource.com/libyuv/libyuv/+/6e498475c749063485a046dfb24e1fa1a81cbedf%5E%21/

Comment on attachment 9010311 [details] Bug 1491848 Update libyuv's preprocessor defines for mingw-clang r=jesup Randell Jesup [:jesup] has approved the revision.

Pushed by shindli@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bd1d38fb6248 Patch libyuv to fix the x86 mingw-clang build r=jesup

[ESR Uplift Approval Request] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed to fix the mingw-clang build for esr60 User impact if declined: Tor will have to carry this patch themselves, and we won't be able to run mingw-clang in automation. Fix Landed on Version: 64.0a1 / 20181004100222 Risk to taking this patch: Medium Why is the change risky/not risky? (and alternatives if risky): Compiler guards that should only change things for MinGW. If they don't; however, the build will almost certainly break. String or UUID changes made by this patch:

Comment on attachment 9024548 [details] [diff] [review] Bug 1491848 Patch libyuv to fix the x86 mingw-clang build (rebased) r=jesup Review of attachment 9024548 [details] [diff] [review]: ----------------------------------------------------------------- r+ though not really needed

Comment on attachment 9024548 [details] [diff] [review] Bug 1491848 Patch libyuv to fix the x86 mingw-clang build (rebased) r=jesup OK for uplift, modifications to the build, useful for downstream maintainers.