Re: 01/01: gnu: fftw: Build SIMD codelets.

[Mark H Weaver <mhw@netris.org>]

Hi Eric,

Mark H Weaver <mhw@netris.org> writes:

> ericbavier@centurylink.net (Eric Bavier) writes:
>
>> bavier pushed a commit to branch core-updates
>> in repository guix.
>>
>> commit 65bb22796f854cbc3eae053a80b1d64365dad376
>> Author: Eric Bavier <bavier@cray.com>
>> Date:   Fri Apr 6 10:53:06 2018 -0500
>>
>>     gnu: fftw: Build SIMD codelets.
>>     
>>     * gnu/packages/algebra.scm (fftw)[arguments]: Remove 'no-native phase; use
>>     configure cache value instead.  Add configure flags for SIMD codelets.
>>     (fftwf)[arguments]: Add neon configuration flag for 32-bit arm.
>>     (fftw-avx): Remove variable.
>
> [...]
>
>> @@ -560,7 +569,10 @@ cosine/ sine transforms or DCT/DST).")
>>      (arguments
>>       (substitute-keyword-arguments (package-arguments fftw)
>>         ((#:configure-flags cf)
>> -        `(cons "--enable-float" ,cf))))
>> +        (if (string-prefix? "arm" (or (%current-target-system)
>> +                                      (%current-system)))
>> +            `(cons "--enable-neon" ,cf)
>> +            cf))))
>
> Did you intend to remove the "--enable-float" configure flag in fftwf?
> You didn't mention this change in the commit log.
>
> The description of fftwf appends "Single-precision version" to the
> description from fftw, but since your commit above, the fftw and fftwf
> packages are identical except on armhf, as far as I can tell.  On armhf,
> the build now fails with "configure: error: NEON requires single
> precision".
>
>   https://hydra.gnu.org/build/2674813/nixlog/1/tail-reload
>
> Can you take a look?

Another problem with the above commit has now become apparent.  In
addition to the 'fftwf' failure at configuration time on armhf, the
'fftw' package also now fails its test suite on our armhf build slaves.
While running the test suite, a test process is killed with SIGILL
(Illegal instruction).

I looked into it, and it appears that the "--enable-armv7a-cntvct"
configure flag that you added should be removed.  Adding it was a good
guess, but apparently the ARMv7-a CNTVCT instruction is not available
from user mode unless you use a specially patched kernel to enable it.

For details, see:

  https://github.com/FFTW/fftw3/blob/master/README-perfcnt.md
  https://github.com/thoughtpolice/enable_arm_pmu
  https://neocontra.blogspot.com/2013/05/user-mode-performance-counters-for.html

Since these issues are delaying the build-out of 'core-updates' on armhf
with ~1K dependency failures, and I'm reasonably confident what needs to
be done here, I'll go ahead and apply untested fixes for these issues to
'core-updates' soon.

     Regards,
       Mark