Re: Fix for INTEL_SSE

On Thu, Oct 20, 2022 at 7:49 PM John Bowler
<[email protected]> wrote:
> The build configuration has to have failed to include intel/intel_init.c!

This was also the case before your patch also, but the build is
passing without your change and it fails with your change.

See, for example:
https://app.travis-ci.com/github/ctruta/libpng/builds/256580435
https://ci.appveyor.com/project/ctruta/libpng/builds/45044129

> From my limited knowledge of cmake  it looks like CMakeLists.txt should include it on line 162 and, indeed, it does on my cmake build.  I just did "cmake ../code" then "make" and the intel files get compiled.

The same is true on my Linux machine, but that's not where the failure is.

> >https://ci.appveyor.com/project/ctruta/libpng/builds/45130105
>
> A similar problem, in fact it looks the same; it would seem that the Intel optimization files have been left out of the build without explicitly turning the optimizations off but I can't work out what options were passed to cmake (even the VS build seems to be using cmake!)

So, what I'm seeing here is that the hardware optimization files were
never picked up, and yet, the build never failed. Now it does fail.

If you want to see the options passed to CMake, see (in the build
logs) the section marked as
## START OF CONFIGURATION ##
and then, a few lines down, something like this:
environment option: $CI_CMAKE_VARS: '-DPNG_HARDWARE_OPTIMIZATIONS=ON'
or, in the VS build, you'll see this:
environment option: $CI_CMAKE_VARS:
'-DCMAKE_TOOLCHAIN_FILE=C:\tools\vcpkg\scripts\buildsystems\vcpkg.cmake'

> Note that all the CMakeLists.txt rules have the same set of bugs; they explicitly turn things on or off.  Why it's going wrong in this case isn't clear to me.  It is very clear that if PNG_INTEL_SSE_OPT is set to 0 (or, indeed, negative) line 236 of pngpriv.h will not be parsed.

You'll need to figure this out, John. I cannot apply a patch that's
regressing the build.

Maybe this helps: a failure that I haven't noticed while writing the
previous email, and it's not with CMake, but with makefile.msys:
https://ci.appveyor.com/project/ctruta/libpng/builds/45130105/job/4p4jaq27xn898b2t
It may be a useful indication that it's failing on Win64 but not on Win32:
https://ci.appveyor.com/project/ctruta/libpng/builds/45130105/job/xkpqbj94535r9551

What's peculiar about makefile.msys is that it neither includes
intel/intel_init.c, nor does it define -DPNG_INTEL_SSE_OPT=0 (like
other makefiles do).

So:

Without your patch:

If you just grab the core source files, without any CPU-specific (and
specifically without intel/intel_init.c and
intel/filter_sse2_intrinsics.c), you can compile everything just fine
on both i686 and x86_64, without any extra macro definition.

With your patch:

You can still compile just the core source files on i686, but you can
no longer do it on x86_64. For that to work, you are now obligated to
pass on -DPNG_INTEL_SSE_OPT=0

I hope this helps.

Sincerely,
Cosmin