1085607 - libvpx doesn't build on OS X with Apple clang from OS X 10.7 command line tools

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Steven Michaud [:smichaud] (Retired)]

The latest (and probably the last) release of Apple's Command Line Tools for OS X Lion (April 2013) contains a copy of clang with the following version information:

$ clang --version
Apple LLVM version 4.2 (clang-425.0.28) (based on LLVM 3.2svn)
Target: x86_64-apple-darwin11.4.2
Thread model: posix

Since bug 1063356 updated libvpx in the tree, the libvpx build now dies (using this version of clang) as follows:

/usr/local/src/Mozilla/asan/mozilla-central/media/libvpx/vp9/common/x86/vp9_subpixel_8t_intrin_avx2.
c:77:18: warning: implicit declaration of function '_mm256_broadcastsi128_si256' is invalid in C99 [
-Wimplicit-function-declaration]
  filtersReg32 = MM256_BROADCASTSI128_SI256(filtersReg);
                 ^
/usr/local/src/Mozilla/asan/mozilla-central/media/libvpx/vp9/common/x86/vp9_subpixel_8t_intrin_avx2.
c:41:41: note: expanded from macro 'MM256_BROADCASTSI128_SI256'
#  define MM256_BROADCASTSI128_SI256(x) _mm256_broadcastsi128_si256(x)
                                        ^
/usr/local/src/Mozilla/asan/mozilla-central/media/libvpx/vp9/common/x86/vp9_subpixel_8t_intrin_avx2.c:77:16: error: assigning to '__m256i' from incompatible type 'int'
  filtersReg32 = MM256_BROADCASTSI128_SI256(filtersReg);
               ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/src/Mozilla/asan/mozilla-central/media/libvpx/vp9/common/x86/vp9_subpixel_8t_intrin_avx2.c:322:16: error: assigning to '__m256i' from incompatible type 'int'
  filtersReg32 = MM256_BROADCASTSI128_SI256(filtersReg);
               ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning and 2 errors generated.

Comment 1

[Steven Michaud [:smichaud] (Retired)]

A similar problem is reported here:

https://code.google.com/p/webm/issues/detail?id=720

I expect this won't be hard to fix.  Hopefully I'll be able to post a patch in the next hour or so.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

I'm not sure those other bugs really are dups of this one.  Or at least I doubt the patch I'm about to post will fix those other bugs.

Comment 5

[Ralph Giles (:rillian)]

Jesup has a work-around patch in bug 1085648.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix

This patch allows me to build on OS X 10.7 (with the version of Apple's clang I mentioned in comment #0).  But I have no idea whether or not it breaks vp9.  Please post a manual testcase, with STR.

For what it's worth, I've started an all-platform tryserver build:
https://tbpl.mozilla.org/?tree=Try&rev=8a5d7a8557bb

I got the following results grepping on _mm_broadcastsi128_si256 in /usr/lib on OS X 10.7.5 and 10.8.5 (after updating to the latest command line tools on 10.8.5):

[10.7.5]
$ grep -r -s _mm_broadcastsi128_si256 *
clang/4.0/include/avx2intrin.h:_mm_broadcastsi128_si256(__m128i const *a)
clang/4.2/include/avx2intrin.h:_mm_broadcastsi128_si256(__m128i const *a)

[10.8.5]
$ grep -r -s _mm_broadcastsi128_si256 *
clang/4.1/include/avx2intrin.h:_mm_broadcastsi128_si256(__m128i const *a)
clang/4.2/include/avx2intrin.h:_mm_broadcastsi128_si256(__m128i const *a)

I think this supports my conclusion (embodied in my patch) that Apple's clang 4.0, 4.1 and 4.2 (Apple's versioning) all support (and need to use) _mm_broadcastsi128_si256.

Comment 7

[Ralph Giles (:rillian)]

Comment on attachment 8508239 [details] [diff] [review]
Fix

Review of attachment 8508239 [details] [diff] [review]:
-----------------------------------------------------------------

Please also add a .patch file to the tree and add it to apply_patches() in update.py.

Otherwise looks fine.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

> also

Do you mean "instead"?

Comment 9

[Ralph Giles (:rillian)]

No, as well as applying the patch; it's to make sure we don't lose the change next time we pull from upstream.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix rev1

How's this?

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

builds for me with this patch on 4.2 (XCode 4.6.3, OSX 10.7.5)

Comment 12

[Ralph Giles (:rillian)]

Comment on attachment 8508282 [details] [diff] [review]
Fix rev1

Review of attachment 8508282 [details] [diff] [review]:
-----------------------------------------------------------------

That's it, thanks. r=me with the patchlevel fixed.

::: media/libvpx/update.py
@@ +530,5 @@
>  def apply_patches():
>      # Patch to permit vpx users to specify their own <stdint.h> types.
>      os.system("patch -p0 < stdint.patch")
> +    # Patch to allow older versions of Apple's clang to build libvpx.
> +    os.system("patch -p0 < apple-clang.patch")

you need -p3 here, so it's relative to the script's parent dir.

Comment 13

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix rev2

What I'll land.  (Carrying forward r+.)

Comment 14

[Steven Michaud [:smichaud] (Retired)]

mozilla-inbound is currently closed.  I'm waiting for it to reopen.

Comment 15

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8508307 [details] [diff] [review]
Fix rev2

Landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5515fa5ba20d

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5515fa5ba20d