1085171 - Can't compile the tree on Mac

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Boris Zbarsky [:bzbarsky]]

I get the following:

 ../../../mozilla/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);
                  ^
 ../../../mozilla/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)
                                         ^
 ../../../mozilla/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);
                ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ../../../mozilla/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);
                ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Bisect says this is a regression from https://hg.mozilla.org/integration/mozilla-inbound/rev/3093663c5c22

Comment 1

[Boris Zbarsky [:bzbarsky]]

Oh, and this is:

% clang --version
clang version 3.4 (trunk 183744)

which is the revision that matches our builders last I checked (though they seem to have a different clang version but the same revision id???).

Comment 2

[Boris Zbarsky [:bzbarsky]]

I'm convinced the versioning thing is screwy.

I have __APPLE__ defined, __clang_major__ is 3, and __clang__minor__ is 4.

So this condition in the ifdefs:

# if __clang_major__ < 3 || (__clang_major__ == 3 && __clang_minor__ <= 3) || \
      (defined(__APPLE__) && __clang_major__ == 5 && __clang_minor__ == 0)

does not test true.  So I end up with _mm256_broadcastsi128_si256 instead of _mm_broadcastsi128_si256.  If I force that condition to match (by throwing in || 1), things compile fine.

I really wish I understood exactly what compiler tinderbox is using here...

Comment 3

[Ralph Giles (:rillian)]

Are you using a custom build of clang? An older xcode?

It does work for me 'Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn)'.

The tinderbox machines use a custom clang binary afaict from the log:

{
"clang_version": "r183744"
},
{
"size": 59602619, 
"digest": "86662ebc0ef650490559005948c4f0cb015dad72c7cac43732c2bf2995247081e30c139cf8008d19670a0009fc302c4eee2676981ee3f9ff4a15c01af22b783b", 
"algorithm": "sha512", 
"filename": "clang.tar.bz2"
},

...but the url it pulls the tarball from doesn't seem to be publicly accessible, so I can't verify.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Are you using a custom build of clang?

Yes.  Specifically, I just pulled snv rev 183744, which is what tinderbox claims to use, and compiled it locally...

Note that Apple's versions for "clang" and the version of "clang" when pulled from the clang repo are different: apple claims version 5.1 when the underlying clang version is actually 3.4-ish, as your output shows.  Similarly, for me if I run the OS-supplied clang I get:

  Apple LLVM version 6.0 (clang-600.0.51) (based on LLVM 3.5svn)

which I'm sure makes the whole __clang_major/minor__ bit very exciting.

This is why I expect the __APPLE__ bit is there, but that's defined in my case.

Comment 5

[Ralph Giles (:rillian)]

(In reply to Boris Zbarsky [:bz] from comment #4)
 
> This is why I expect the __APPLE__ bit is there, but that's defined in my
> case.

I suspect the code is trying to distinguish Apple clang (__APPLE__ being a platform define) and upstream clang on non-apple platforms (Linux) and check the appropriate versions for each.

So now I'm wondering how our custom toolchain works on the tinderbox...

Comment 6

[Ralph Giles (:rillian)]

smichaud suggests it's an issue with the last supported Xcode on MacOS X 10.7.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I'm on OS X 10.9.  I guess we'll see whether bug 1085607 fixes this issue....

Comment 8

[Mike Hommey [:glandium]]

(In reply to Boris Zbarsky [:bz] from comment #4)
> > Are you using a custom build of clang?
> 
> Yes.  Specifically, I just pulled snv rev 183744, which is what tinderbox
> claims to use, and compiled it locally...

In case it makes a difference, try building it with build/unix/build-clang/build-clang.py

Comment 9

[Boris Zbarsky [:bzbarsky]]

The patch for bug 1085607 doesn't fix this issue.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> In case it makes a difference, try building it with build/unix/build-clang/build-clang.py

Ah, I see (though actually figuring out how to use this script is ... exciting).  This is actually pulling a 3.3 branch revision, not a trunk revision, but they happen to have the same revision ids...

Which if you look at comment 2 falls in the "(__clang_major__ == 3 && __clang_minor__ <= 3)" bucket, so works correctly.

Comment 11

[Mike Hommey [:glandium]]

Note there's a clang-trunk.json file, too.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Right; I'm mostly interested in using the same clang as out CI infrastructure, which seems to be the branch one there.

Comment 13

[Ralph Giles (:rillian)]

(In reply to Boris Zbarsky [:bz] from comment #10)

> Ah, I see (though actually figuring out how to use this script is ...
> exciting).  This is actually pulling a 3.3 branch revision, not a trunk
> revision, but they happen to have the same revision ids...

Right, svn revision ids don't uniquely identify a version; one needs the repository path as well. I guess that explains the difference.

If you're ok with the build-clang.py version, can we consider this resolved?

Comment 14

[Boris Zbarsky [:bzbarsky]]

Yeah, thank you!