<div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Dear Andreas and others, </div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Thanks a lot for your help in fixing the compatibility issues in IQ-TREE. We will apply the patches to our GIT repository. </div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Cheers</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Tung </div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 29, 2016 at 2:16 PM, Andreas Tille <span dir="ltr"><<a href="mailto:andreas@an3as.eu" target="_blank">andreas@an3as.eu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Hi Tung,<br>
<br>
thanks to the very helpful and detailed response of Christian Seiler<br>
which I would like you to read below in detail since it explains the<br>
patches applied in the Debian packaging I was able to build the latest<br>
iqtree version.  You can find all patches that are applied to iqtree<br>
here:<br>
<br>
    <a href="https://anonscm.debian.org/cgit/debian-med/iqtree.git/tree/debian/patches" rel="noreferrer" target="_blank">https://anonscm.debian.org/cgit/debian-med/iqtree.git/tree/debian/patches</a><br>
<br>
Please also note that I have updated the spelling patch with some<br>
spelling mistakes (in code and comments).<br>
<br>
Kind regards and thanks for your cooperation<br>
<br>
     Andreas.<br>
<br>
On Tue, Jun 28, 2016 at 08:09:15PM +0200, Christian Seiler wrote:<br>
> On 06/28/2016 11:01 AM, Andreas Tille wrote:<br>
> > I admit I can not answer the question asked by upstream.  The package in<br>
> > question is iqtree[1] and they said that they have different<br>
> > computational kernels implemented to respect different hardware.<br>
> > Current Git[1] does not even build - may be due to some fine tuning of<br>
> > gcc options needed???<br>
><br>
> I've looked at this, and there are a couple of things going on here:<br>
><br>
> 0. Debian's build flags by default assume a generic architecture, so<br>
> you don't have to do anything by yourself.<br>
><br>
> 1. Upstream's build system supports multiple options for the entirety<br>
> of the code. So you can compile the entire code with the AVX or FMA<br>
> instruction set. You patch that out completely from the CMakeLists.txt<br>
> in sse.patch, but that isn't actually required. (IQTREE_FLAGS would<br>
> have to be explicitly set to enable this.)<br>
><br>
> 2. Furthermore, upstream's build system provides SSE and AVX kernels<br>
> for regardless of the build flags of the rest of the code, and they are<br>
> always compiled. (Well, you can disable compilation of the AVX kernel<br>
> if you add "novx" to IQTREE_FLAGS, but there's no reason to.) This<br>
> should work out of the box.<br>
><br>
> That said, the code doesn't support non-SSE at all, because it hard-<br>
> codes at least SSE2 intrinsics in a lot of platces (and the one part<br>
> where it hardcodes SSE3, you already have a patch for). The code can<br>
> therefore not be compiled without SSE support enabled, unfortunately,<br>
> even on i386. If you want to support non-SSE at all on i386, upstream<br>
> (or yourself) needs to implement the routines in the vectorclass/<br>
> directory (and possibly others) for non-SSE systems. (The kernel that<br>
> optionally uses AVX also exists in a non-SSE variant, so upstream is<br>
> not completely wrong about that, but there's a lot of _other_ code that<br>
> forces at least SSE2.<br>
><br>
> 3. pll/ has a bug that it calls posix_memalign with PLL_BYTE_ALIGNMENT.<br>
> However, according to the manpage, the alignment must be a multiple of<br>
> sizeof(void *) for posix_memalign to work (and a power of 2), but<br>
> PLL_BYTE_ALIGNMENT is 1 if SSE3 is not used. If you explicitly set it<br>
> to 8 (to catch both 32bit and 64bit), posix_memalign will not fail and<br>
> the program won't segfault anymore. (posix_memalign with wrong align<br>
> argument will just return without a possibility to check for an error,<br>
> but also not allocating a buffer, leaving it empty.)<br>
><br>
> Note though that if you don't compile with sse3 flags enabled, pll will<br>
> not use SSE code at all (other than that which the compiler generates),<br>
> which is probably slower. But it does work, though. (A grep for __SSE3<br>
> shows though that porting this would be a LOT of work.)<br>
><br>
> Irrespective of the SSE-stuff, two things:<br>
><br>
> 1. Your debian/rules calls dh_auto_clean/configure/build in<br>
>    override_dh_auto_build to build two variants. This can be done in a<br>
>    more elegant way, because CMake does support out of tree builds, and<br>
>    you can have debhelper use a specific build directory by specifying<br>
>    -Bdirname to dh_auto_*.<br>
><br>
> 2. You might want to add --parallel to your dh call in debian/rules.<br>
>    CMake-based projects tend to support paralle builds, and iqtest is<br>
>    no exception to that rule. Would speed up build times quite a bit.<br>
><br>
> 3. If you want to test the -omp binary as you do in debian/rules<br>
>    currently, you have to pass -redo, otherwise the second call will<br>
>    simply fail.<br>
><br>
> I've update your sse.patch to include the SSE-related fixes, and have<br>
> updated debian/rules to incorporate the two other things. Attached both<br>
> to this email. The package now builds on amd64 and i386 (and probably<br>
> will build on the kfreebsd and hurd variants thereof, though I haven't<br>
> checked) and the test suite runs. The AVX/FMA checks in CMakeLists.txt<br>
> are now not removed, because debian/rules never sets IQTEST_FLAGS to<br>
> fma or avx. (On amd64 the avx kernel is built with -mavx regardless<br>
> separately by the build system, so that's also OK; and on my Haswell<br>
> system the AVX detection works. On i386 the AVX kernel is never built,<br>
> as per what the upstream build system decided.) However, even on i386,<br>
> SSE2 support is required for this to work, otherwise the program will<br>
> crash with either illegal instruction or a segfault at start. (I can<br>
> provide you with a preinst script that checks for SSE2 support to show<br>
> a nice error message at package installation time, if you so wish.)<br>
><br>
> Additionally (what I've NOT done): please check the lintian info and<br>
> pedantic messages of the package:<br>
><br>
>  - out-of-date-standards-version 3.9.7<br>
>  - a couple of spelling-error-in-binary<br>
>  - hardening-no-pie/hardening-no-bindnow: consider enabling all<br>
>    hardening flags (if that works, haven't checked)<br>
>  - copyright-refers-to-symlink-license: you should refer to<br>
>    /usr/share/common-licenses/GPL-3 and not .../GPL in the GPL-3<br>
>    block of debian/copyright<br>
><br>
> Hope that helps.<br>
><br>
> Regards,<br>
> Christian<br>
<br>
> #!/usr/bin/make -f<br>
><br>
> # DH_VERBOSE := 1<br>
><br>
> pkg := $(shell dpkg-parsechangelog | sed -n 's/^Source: //p')<br>
> version=$(shell dpkg-parsechangelog -ldebian/changelog | grep Version: | cut -f2 -d' ' | cut -f1 -d- )<br>
> mandir=$(CURDIR)/debian/$(pkg)/usr/share/man/man1/<br>
><br>
> %:<br>
>       dh $@ --parallel<br>
><br>
> VARIANTS = omp serial<br>
><br>
> override_dh_auto_configure: $(foreach variant,$(VARIANTS),dh_auto_configure_$(variant))<br>
> override_dh_auto_build:     $(foreach variant,$(VARIANTS),dh_auto_build_$(variant))<br>
> override_dh_auto_install:   $(foreach variant,$(VARIANTS),dh_auto_install_$(variant))<br>
> override_dh_auto_clean:     $(foreach variant,$(VARIANTS),dh_auto_clean_$(variant))<br>
><br>
> dh_auto_configure_omp:<br>
>       dh_auto_configure -Bbuild.omp -- -DIQTREE_FLAGS="omp"<br>
><br>
> dh_auto_configure_serial:<br>
>       dh_auto_configure -Bbuild.serial -- -DIQTREE_FLAGS=""<br>
><br>
> dh_auto_build_%:<br>
>       dh_auto_build -Bbuild.$(subst dh_auto_build_,,$@)<br>
><br>
> dh_auto_install_%:<br>
>       dh_auto_install -Bbuild.$(subst dh_auto_install_,,$@)<br>
><br>
> dh_auto_clean_%:<br>
>       dh_auto_clean -Bbuild.$(subst dh_auto_clean_,,$@)<br>
><br>
> override_dh_installexamples:<br>
>       dh_installexamples<br>
>       # remove example files in unusual dir<br>
>       rm -f debian/*/usr/models.nex<br>
>       rm -f debian/*/usr/example.[np][eh][xy]<br>
><br>
> override_dh_installman:<br>
>       mkdir -p $(mandir)<br>
>       help2man --no-info --no-discard-stderr --help-option="-h" \<br>
>           --name='efficient phylogenetic software by maximum likelihood' \<br>
>           --version-string="$(version)" $(CURDIR)/debian/$(pkg)/usr/bin/iqtree > $(mandir)/iqtree.1<br>
>       help2man --no-info --no-discard-stderr --help-option="-h" \<br>
>           --name='efficient phylogenetic software by maximum likelihood (multiprocessor version)' \<br>
>           --version-string="$(version)" $(CURDIR)/debian/$(pkg)/usr/bin/iqtree-omp > $(mandir)/iqtree-omp.1<br>
><br>
> override_dh_auto_test:<br>
>       # use only the first example for build time test to save time on autobuilders<br>
> #     if [ "`find $(CURDIR) -name iqtree -type f -executable`" = "" ] ; then \<br>
> #             iqtreeomp=`find $(CURDIR) -name iqtree-omp -type f -executable` ; \<br>
> #             ln -s iqtree-omp `dirname $$iqtreeomp`/iqtree ; \<br>
> #     fi<br>
>       sed '/ myprefix/,$$d' debian/Documents_source/example.sh > example.short<br>
>       echo 'time $(CURDIR)/build.omp/iqtree-omp -s example.phy -omp 2 -redo' >> example.short<br>
>       time sh example.short<br>
>       rm example.short<br>
<br>
> Description: Do not use -m32 and -msse3 flags<br>
> Bug-Debian: <a href="https://bugs.debian.org/813436" rel="noreferrer" target="_blank">https://bugs.debian.org/813436</a><br>
> Author: Andreas Tille <<a href="mailto:tille@debian.org">tille@debian.org</a>><br>
> Last-Update: Tue, 02 Feb 2016 08:41:45 +0100<br>
><br>
> --- a/CMakeLists.txt<br>
> +++ b/CMakeLists.txt<br>
> @@ -1,4 +1,4 @@<br>
> -##################################################################<br>
> +     ##################################################################<br>
>  # IQ-TREE cmake build definition<br>
>  # Copyright (c) 2012-2015 Bui Quang Minh, Lam Tung Nguyen<br>
>  ##################################################################<br>
> @@ -172,7 +172,7 @@ if(CMAKE_SIZEOF_VOID_P EQUAL 4 OR IQTREE<br>
>       endif()<br>
>       SET(EXE_SUFFIX "${EXE_SUFFIX}32")<br>
>       if (GCC OR CLANG)<br>
> -             set(COMBINED_FLAGS "${COMBINED_FLAGS} -m32")<br>
> +             set(COMBINED_FLAGS "${COMBINED_FLAGS}")<br>
>       endif()<br>
>      add_definitions(-DBINARY32)<br>
>  else()<br>
> @@ -237,7 +237,7 @@ SET(SSE_FLAGS "")<br>
>  if (VCC)<br>
>       set(SSE_FLAGS "/arch:SSE2 -D__SSE3__")<br>
>  elseif (GCC OR CLANG)<br>
> -     set(SSE_FLAGS "-msse3")<br>
> +     set(SSE_FLAGS "-msse2")<br>
>  elseif (ICC)<br>
>       if (WIN32)<br>
>               set(SSE_FLAGS "/arch:SSE3")<br>
> @@ -273,8 +273,7 @@ elseif (IQTREE_FLAGS MATCHES "avx") # AV<br>
><br>
>       SET(EXE_SUFFIX "${EXE_SUFFIX}-avx")<br>
>  else() #SSE intruction set<br>
> -     message("Vectorization : SSE3")<br>
> -     add_definitions(-D__SSE3)<br>
> +     message("Vectorization : SSE2")<br>
><br>
>  endif()<br>
><br>
> --- a/phylokernel.h<br>
> +++ b/phylokernel.h<br>
> @@ -15,6 +15,10 @@<br>
>  inline Vec2d horizontal_add(Vec2d x[2]) {<br>
>  #if  INSTRSET >= 3  // SSE3<br>
>      return _mm_hadd_pd(x[0],x[1]);<br>
> +#elif   INSTRSET >= 2  // SSE3<br>
> +    Vec2d help0 = _mm_shuffle_pd(x[0], x[1], _MM_SHUFFLE2(0,0));<br>
> +    Vec2d help1 = _mm_shuffle_pd(x[0], x[1], _MM_SHUFFLE2(1,1));<br>
> +    return _mm_add_pd(help0, help1);<br>
>  #else<br>
>  #error "You must compile with SSE3 enabled!"<br>
>  #endif<br>
> --- a/pll/pll.h<br>
> +++ b/pll/pll.h<br>
> @@ -82,7 +82,7 @@ extern "C" {<br>
>  #define PLL_VECTOR_WIDTH 2<br>
><br>
>  #else<br>
> -#define PLL_BYTE_ALIGNMENT 1<br>
> +#define PLL_BYTE_ALIGNMENT 8<br>
>  #define PLL_VECTOR_WIDTH 1<br>
>  #endif<br>
><br>
<span class=""><font color="#888888"><br>
<br>
--<br>
<a href="http://fam-tille.de" rel="noreferrer" target="_blank">http://fam-tille.de</a><br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div dir="ltr" style="color:rgb(136,136,136)"><div dir="ltr"><div dir="ltr">Dipl.-Ing. Tung Nguyen, PhD</div><div dir="ltr">Center for Integrative Bioinformatics Vienna (CIBIV)</div><div dir="ltr">Max F. Perutz Laboratories (MFPL)</div><div dir="ltr">Campus Vienna Biocenter 5 (VBC5), Ebene 1, Room 1812.4</div><div dir="ltr">A-1030 Wien, Austria</div><div dir="ltr">Phone: +43 +1 / 42777-24025</div><div dir="ltr">Handy: +4369911551566</div><div dir="ltr">Email:   <a href="mailto:tung.nguyen@univie.ac.at" target="_blank">tung.nguyen@univie.ac.at</a></div></div></div></div></div></div></div>
</div></div>