Closed Bug 1085607 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Audio/Video, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: smichaud, Assigned: smichaud)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
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.
Duplicate of this bug: 1085599
Duplicate of this bug: 1085171
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.
Jesup has a work-around patch in bug 1085648.
See Also: → 1085648
Attached patch Fix (obsolete) — Splinter Review
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.
Assignee: nobody → smichaud
Attachment #8508239 - Flags: review?(giles)
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.
Attachment #8508239 - Flags: review?(giles) → review-
> also

Do you mean "instead"?
No, as well as applying the patch; it's to make sure we don't lose the change next time we pull from upstream.
Attached patch Fix rev1 (obsolete) — Splinter Review
How's this?
Attachment #8508239 - Attachment is obsolete: true
Attachment #8508282 - Flags: review?(giles)
builds for me with this patch on 4.2 (XCode 4.6.3, OSX 10.7.5)
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.
Attachment #8508282 - Flags: review?(giles) → review+
Attached patch Fix rev2Splinter Review
What I'll land.  (Carrying forward r+.)
Attachment #8508282 - Attachment is obsolete: true
Attachment #8508307 - Flags: review+
mozilla-inbound is currently closed.  I'm waiting for it to reopen.
https://hg.mozilla.org/mozilla-central/rev/5515fa5ba20d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.