[FFmpeg-devel] [PATCH] all: Replace if (ARCH_FOO) checks by #if ARCH_FOO

[Martin Storsjö]

On Sun, 12 Jun 2022, Martin Storsjö wrote:

> On Sun, 12 Jun 2022, Soft Works wrote:
>
>> 
>> 
>>> -----Original Message-----
>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>> Andreas Rheinhardt
>>> Sent: Sunday, June 12, 2022 7:28 AM
>>> To: ffmpeg-devel at ffmpeg.org
>>> Cc: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>> Subject: [FFmpeg-devel] [PATCH] all: Replace if (ARCH_FOO) checks by
>>> #if ARCH_FOO
>>> 
>>> This is more spec-compliant because it does not rely
>>> on dead-code elimination by the compiler. Especially
>>> MSVC has problems with this, as can be seen in
>>> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296373.html
>>> or
>>> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/297022.html
>>> 
>>> This commit does not eliminate every instance where we rely
>>> on the dead code elimination: It only tackles branching to
>>> the initialization of arch-specific dsp code, not e.g. all
>>> uses of CONFIG_ and HAVE_ checks. But maybe it is already
>>> enough to compile FFmpeg with MSVC with whole-programm-optimizations
>>> enabled (if one does not disable too many components).
>>> 
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>> ---
>> 
>> LGTM.
>> 
>> It's not really a story as simple as "poor MSVC is unable
>> to perform dead-code-elimination". It is actually capable to do that,
>> but the ffmpeg code was not only requiring the compiler to eliminate
>> dead code,
>
>> it actually required a compiler to ignore dead code blocks
>> even when those would contain invalid code that cannot be compiled
>> at all.
>
> Can you qualify this statement? This does not match my understanding.

... because eliminating unused code (for e.g. debug printouts) using DCE 
instead of ifdefs has got the advantage that the skipped code still is 
syntax checked (so it doesn't bitrot), compared to in ifdefs where it can 
easily become stale.

Still not an argument against this patch, only against seemingly incorrect 
statements.

// Martin