Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 25 Aug 2019 15:23:32 -0600
From:      Adam Weinberger <adamw@adamw.org>
To:        Yuri Victorovich <yuri@freebsd.org>
Cc:        ports-committers <ports-committers@freebsd.org>, svn-ports-all <svn-ports-all@freebsd.org>,  svn-ports-head <svn-ports-head@freebsd.org>
Subject:   Re: svn commit: r509806 - head/math/blasfeo
Message-ID:  <CAP7rwcg_OtcNH1DqPLzsA%2B878C2FY1WK35vTV3oaL3%2BAhswVWw@mail.gmail.com>
In-Reply-To: <201908251703.x7PH3bbw038772@repo.freebsd.org>
References:  <201908251703.x7PH3bbw038772@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Aug 25, 2019 at 11:03 AM Yuri Victorovich <yuri@freebsd.org> wrote:
>
> Author: yuri
> Date: Sun Aug 25 17:03:37 2019
> New Revision: 509806
> URL: https://svnweb.freebsd.org/changeset/ports/509806
>
> Log:
>   math/blasfeo: Specify TARGET in architecture-specific way, this fixes build on non-Intel architectures
>
>   Reported by:  fallout
>
> Modified:
>   head/math/blasfeo/Makefile
>
> Modified: head/math/blasfeo/Makefile
> ==============================================================================
> --- head/math/blasfeo/Makefile  Sun Aug 25 16:55:07 2019        (r509805)
> +++ head/math/blasfeo/Makefile  Sun Aug 25 17:03:37 2019        (r509806)
> @@ -2,6 +2,7 @@
>
>  PORTNAME=      blasfeo
>  DISTVERSION=   0.1.1
> +PORTREVISION=  1
>  CATEGORIES=    math
>
>  MAINTAINER=    yuri@FreeBSD.org
> @@ -13,5 +14,23 @@ LICENSE_FILE=        ${WRKSRC}/LICENSE.txt
>  USES=          cmake
>  USE_GITHUB=    yes
>  GH_ACCOUNT=    giaf
> +
> +CMAKE_ARGS=    -DTARGET:STRING=GENERIC
> +
> +OPTIONS_SINGLE=                ${ARCH:C/amd64/TARGET/:C/[a-z].*//} # architecture-wise options are based on the list in Makefile.rule
> +OPTIONS_SINGLE_TARGET= ${ARCH:C/amd64/GENERIC X64_INTEL_HASWELL X64_INTEL_SANDY_BRIDGE X64_INTEL_CORE X64_AMD_BULLDOZER/:C/[a-z].*//}
> +OPTIONS_DEFAULT=       ${ARCH:C/amd64/GENERIC/:C/[a-z].*//}
> +# TODO ARM also has SIMD acceleration
> +
> +GENERIC_DESC=                          Generic C code without SIMD acceleration
> +X64_INTEL_HASWELL_DESC=                        x86_64 architecture with AVX2 and FMA ISA (64 bit OS)
> +X64_INTEL_SANDY_BRIDGE_DESC=           x86_64 architecture with AVX ISA (64 bit OS)
> +X64_INTEL_CORE_DESC=                   x86_64 architecture with SSE3 (64 bit OS)
> +X64_AMD_BULLDOZER_DESC=                        x86_64 architecture with AVX and FMA ISA (64 bit OS)
> +
> +X64_INTEL_HASWELL_CMAKE_ON=            -DTARGET:STRING=X64_INTEL_HASWELL
> +X64_INTEL_SANDY_BRIDGE_CMAKE_ON=       -DTARGET:STRING=X64_INTEL_SANDY_BRIDGE
> +X64_INTEL_CORE_CMAKE_ON=               -DTARGET:STRING=X64_INTEL_CORE
> +X64_AMD_BULLDOZER_CMAKE_ON=            -DTARGET:STRING=X64_AMD_BULLDOZER
>
>  .include <bsd.port.mk>

Hi Yuri,

This is clever, but it doesn't feel like a good use of OPTIONS. If a
builder CPU rev changes, the package changes, which prevents some
reproducibility. Dynamic, per-host OPTIONS changes are not what the
OPTIONS paradigm generally does.

The way this kind of thing is normally handled is by adding an
OPTIMIZED_CFLAGS option, which could then contain the logic you have
there.

If people really will want to pick a target they don't run, then keep
them as options but still make GENERIC the default. If people will
only want to build for the target they're running, then this should
probably be implemented as a CMakeLists.txt patch.

# Adam


-- 
Adam Weinberger
adamw@adamw.org
https://www.adamw.org



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAP7rwcg_OtcNH1DqPLzsA%2B878C2FY1WK35vTV3oaL3%2BAhswVWw>