1298418 - clang-cl warning/error: _tzcnt_u32 was not declared

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[Nathan Froyd [:froydnj]]

clang-cl complains:

11:33.48 c:/m-c/media/ffvpx/libavutil/x86/intmath.h(59,33):  warning: implicit declaration of function '_tzcnt_u32' is invalid in C99 [-Wimplicit-function-declaration]
11:33.48     return ((uint32_t)v == 0) ? _tzcnt_u32((uint32_t)(v >> 32)) + 32 : _tzcnt_u32((uint32_t)v);
11:33.48                                 ^

which then leads to linking failures later:

11:34.10 mathematics.obj : error LNK2019: unresolved external symbol __tzcnt_u32 referenced in function _av_gcd
11:34.10 
11:34.10 mozavutil.dll : fatal error LNK1120: 1 unresolved externals
11:34.10 

as _tzcnt_u32 was expected to be inlined.

The problem here is that clang-cl only makes intrinsics available based on the compilation flags you have selected, whereas MSVC makes all the intrinsics available all the time, cf https://llvm.org/bugs/show_bug.cgi?id=25544 , which is a WONTFIX on the clang-cl side.  Chromium had to work around this too; from reading the bug report below, I think the problem was fixed upstream, and then they imported a new version of the source:

http://crbug.com/556735

Can we upgrade our version of ffmpeg to include the relevant fix, or patch our version of ffmpeg?

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

You can clear MOZ_FFVPX in configure in the meanwhile.

Comment 2

[Jean-Yves Avenard [:jya]]

We use ffmpeg 3.1 as of July 2016, which is a much later version than the one mentioned in the chromium bug

Comment 3

[Nathan Froyd [:froydnj]]

Ah, apparently ffmpeg broke things a while back:

https://github.com/FFmpeg/FFmpeg/commit/f98417451291a3ff6719d739b5e904e0b7bf404b

and chromium has not yet updated to a newer version.  Do we have a mechanism for providing local patches, so we can make this work in clang-cl?

Comment 4

[Jean-Yves Avenard [:jya]]

Well, we don't really. we've been trying to be as close to 100% identical to upstream.

It would be best to have the past accepted upstream and then we resync.

They are very quick at accepting new patches like this, and I can do the resync as soon you you ping me that it's done...

Comment 5

[Jean-Yves Avenard [:jya]]

Do you have a patch handy for this?

could submit it upstream for you.

Comment 6

[Nathan Froyd [:froydnj]]

(In reply to Jean-Yves Avenard [:jya] from comment #5)
> Do you have a patch handy for this?
> 
> could submit it upstream for you.

I do, but clang-cl is also working on fixing it on their side, albeit requiring special compilation options.  But the patch to fix it has been waiting more than a week with no movement on the review front...so I'm not sure what the best course of action here is.

Comment 7

[Nathan Froyd [:froydnj]]

Attachment: Bug 1298418 - use clang modules with ffvpx's libavutil and clang-cl;

clang-cl's <intrin.h> is not completely compatible with MSVC's
<intrin.h> by default, as clang-cl's version does not include all the
Intel intrinsic headers by default.  clang-cl does this to make
<intrin.h> as small as possible, as this was shown to have significant
wins for compile time.  Compiling with modules, however, includes all
the Intel intrinsic headers by default, so while upstream decides on a
general solution, we need to compile with modules locally.

Review commit: https://reviewboard.mozilla.org/r/96644/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96644/

Comment 8

[Michael Shal [:mshal]]

Comment on attachment 8815868 [details]
Bug 1298418 - use clang modules with ffvpx's libavutil and clang-cl;

https://reviewboard.mozilla.org/r/96644/#review97266

Are you still planning to remove this if it gets fixed upstream? Can you get a followup on file if so?

Comment 9

[Nathan Froyd [:froydnj]]

(In reply to Michael Shal [:mshal] from comment #8)
> Are you still planning to remove this if it gets fixed upstream? Can you get
> a followup on file if so?

Yes.  Bug 1321651 tracks.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a63ed01b4c56
use clang modules with ffvpx's libavutil and clang-cl; r=mshal

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a63ed01b4c56