[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Packaging xsimd

As far as --march=native goes, in my opinion it's ok since it's only used in tests, i.e. testing that the files build and run. This is because it's actually a header-only package, no binaries (executables or libraries) are provided. So the package that we ship hasn't been "built" with --march=native, it's only been tested with it.
I suggest not explicitly banning the failing arches at this point. They shouldn't hold up migration to testing (after the stable release, that is) since they haven't previously been provided. The migration ban on failed builds applies when a package has previously built successfully, so it doesn't apply to new packages. Having the build logs for failed builds can be useful for tracking what their problem is, and monitor progress when the builds are fixed.
The exception might be if the package explicitly pulls in machine language for specific arches, which can be done from C, in which case the package is designed to not build on other arches. In that case it makes more sense to ban the failing arches, if it's known they can never work (until explicitly ported).
On that note, not all arches have the --march=native flag anyway. CMakeFiles.txt in xtensor tests if the flag is available when configuring tests. So other arches might build if that test and that conditional configuration is patched in. 

Drew


On 2021-04-22 19:30, Nilesh Patra wrote:

On Thu, 22 Apr 2021 at 22:06, Julien Lamy <lamy@unistra.fr> wrote:


Le 22/04/2021 à 18:29, Nilesh Patra a écrit :



On Thu, 22 Apr 2021 at 21:38, Julien Lamy <lamy@unistra.fr
<mailto:lamy@unistra.fr>> wrote:

No, it can work with only SSE and only SSE2 enabled, which

would match

the baseline (tested with a non-AVX machine, I could not get

my

hands on
a non-SSE2 box).



* Is this portable to arches other than x86 and arm?


Theoretically yes: there is a fallback mode which codes the

SIMD

instructions as loops. I have not tested it in a non-x86 and

non-ARM

environment.


I will try testing it. It looks unlikely since it seems to need a

native

architecture. Likely build time tests will
not work, but I'll check nevertheless


Thanks, I'll be curious of the results.


I tried in a ppc64el porter box, and I get several of:

/home/nilesh/xsimd/xsimd/test/test_batch_bool.cpp:315:1: error:
'gtest_type_params_batch_bool_test_' was not declared in this scope;
did you mean 'gtest_type_params_batch_bool_test_NameGenerator'?
  315 | TYPED_TEST(batch_bool_test, load_store)
      | ^~~~~~~~~~
/home/nilesh/xsimd/xsimd/test/test_batch_bool.cpp:315:1: error:
template argument 3 is invalid
  315 | TYPED_TEST(batch_bool_test, load_store)
      | ^~~~~~~~~~
/home/nilesh/xsimd/xsimd/test/test_batch_bool.cpp:315:1: error:
'gtest_type_params_batch_bool_test_' was not declared in this scope;
did you mean 'gtest_type_params_batch_bool_test_NameGenerator'?
  315 | TYPED_TEST(batch_bool_test, load_store)
      | ^~~~~~~~~~

And a failing build. Both for build time as well as autopkgtests.
Do you think we should for now limit arches to amd64 i386 and arm64 in
d/control for now?




* Readme has instructions to build documentation, and you

have added

Build Depends as well, to build it. However they are

neither

built nor

installed.
    If you think building and installing docs make sense,

could

you fix

it? Please install docs in a separate binary package if so.


Done.

I've also bumped the version to 7.5.0, released yesterday

while I was

packaging :)


I do not see your changes on salsa[1] - did you forget to push in

any case?

Yes :( Fixed now.


I have following comments to make:

* Why is the package named xsimd-dev instead of libxsimd-dev? It might
match xtensor, but AFAICS that's
against the library style packaging. For ref:
https://www.netfort.gr.jp/~dancer/column/libpkg-guide/libpkg-guide.html

* Please commit v7.5.0 to pristine-tar

* Some files in ./include have excerpts from code that belongs to
Boost Software license. For example:
./include/xsimd/math/xsimd_error.hpp
   This should be mentioned in d/cpopyright
   Also, this file: ./test/test_constant_batch.cpp has a different
copyright holder "Serge Guelton and QuantStack" so this should also be
mentioned explicitly with
   Files: ./test/test_constant_batch.cpp
   Copyright: Serge Guelton and QuantStack
   ..............
   This is not exhaustive, please consider doing a scrutiny for the
entire codebase and mention copyrights for any files that differ/have
different license

* Autopkgtest on salsa CI fails, consider fixing it:
https://salsa.debian.org/science-team/xsimd/-/jobs/1599080

Nilesh
Reply to: