As David noticed in [1] there is an issue with C++ compilation of the rte_vect.h header in RISC-V. Upon closer inspection, the problem appears on all architectures due to the type conversion rules in C++. More precisely a union type rte_xmm_t requires a conversion constructor from xmm_t type. The most obvious fix is to use a structure initializer for such copies (since rte_xmm_t union contains xmm_t anyway). The generated assembly at -O2 is exactly the same, so there's no real impact. The bigger question is whether accessing bits of the architecture specific xmm_t type in an array fashion is always correct? All current architectures define rte_xmm_t in the same manner implying that. Additionally change RISC-V CI settings to use crossbuild-essential-riscv64 package which provides tools that enable C++ checks. [1] http://mails.dpdk.org/archives/dev/2022-June/243683.html Stanislaw Kardach (3): eal/riscv: fix xmm_t casting for C++ lpm: fix xmm_t casting for C++ in scalar version ci: use crossbuild-essential-riscv64 for compiling .github/workflows/build.yml | 3 +-- lib/eal/riscv/include/rte_vect.h | 4 ++-- lib/lpm/rte_lpm_scalar.h | 11 ++++++----- 3 files changed, 9 insertions(+), 9 deletions(-) -- 2.30.2
rte_xmm_t is a union type which wraps around xmm_t and maps its contents to scalar structures. Since C++ has stricter type conversion rules than C, the rte_xmm_t::x has to be used instead of C-casting. Signed-off-by: Stanislaw Kardach <kda@semihalf.com> --- lib/eal/riscv/include/rte_vect.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/eal/riscv/include/rte_vect.h b/lib/eal/riscv/include/rte_vect.h index 4600521c20..2f97f437a2 100644 --- a/lib/eal/riscv/include/rte_vect.h +++ b/lib/eal/riscv/include/rte_vect.h @@ -41,8 +41,8 @@ vect_load_128(void *p) static inline xmm_t vect_and(xmm_t data, xmm_t mask) { - rte_xmm_t ret = (rte_xmm_t)data; - rte_xmm_t m = (rte_xmm_t)mask; + rte_xmm_t ret = {.x = data }; + rte_xmm_t m = {.x = mask }; ret.u64[0] &= m.u64[0]; ret.u64[1] &= m.u64[1]; return ret.x; -- 2.30.2
rte_xmm_t is a union type which wraps around xmm_t and maps its contents to scalar structures. Since C++ has stricter type conversion rules than C, the rte_xmm_t::x has to be used instead of C-casting. The generated assembly is identical to the code without the fix (checked both on x86 and RISC-V). Signed-off-by: Stanislaw Kardach <kda@semihalf.com> --- lib/lpm/rte_lpm_scalar.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/lpm/rte_lpm_scalar.h b/lib/lpm/rte_lpm_scalar.h index f0d9f37894..161b40ff80 100644 --- a/lib/lpm/rte_lpm_scalar.h +++ b/lib/lpm/rte_lpm_scalar.h @@ -15,18 +15,19 @@ extern "C" { static inline void rte_lpm_lookupx4(const struct rte_lpm *lpm, xmm_t ip, uint32_t hop[4], - uint32_t defv) + uint32_t defv) { + rte_xmm_t xip = { .x = ip }; uint32_t nh; int ret; - ret = rte_lpm_lookup(lpm, ((rte_xmm_t)ip).u32[0], &nh); + ret = rte_lpm_lookup(lpm, xip.u32[0], &nh); hop[0] = (ret == 0) ? nh : defv; - ret = rte_lpm_lookup(lpm, ((rte_xmm_t)ip).u32[1], &nh); + ret = rte_lpm_lookup(lpm, xip.u32[1], &nh); hop[1] = (ret == 0) ? nh : defv; - ret = rte_lpm_lookup(lpm, ((rte_xmm_t)ip).u32[2], &nh); + ret = rte_lpm_lookup(lpm, xip.u32[2], &nh); hop[2] = (ret == 0) ? nh : defv; - ret = rte_lpm_lookup(lpm, ((rte_xmm_t)ip).u32[3], &nh); + ret = rte_lpm_lookup(lpm, xip.u32[3], &nh); hop[3] = (ret == 0) ? nh : defv; } -- 2.30.2
The current packages installed for RISC-V build check do not contain a C++ compiler, which hid an issue with C++ type conversion in the rte_vect.h header on RISC-V or in the scalar implementation of the LPM x4 lookup. Now that this issue is fixed, use the full toolchain install to enable the C++ test. Besides, the user's guide for RISC-V cross-compilation recommends the use of crossbuild-essential-riscv64. Signed-off-by: Stanislaw Kardach <kda@semihalf.com> --- .github/workflows/build.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7c8528cb04..c0d2829d0e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -139,8 +139,7 @@ jobs: pkg-config-powerpc-linux-gnu - name: Install riscv64 cross compiling packages if: env.RISCV64 == 'true' - run: sudo apt install -y gcc-riscv64-linux-gnu libc6-dev-riscv64-cross - pkg-config-riscv64-linux-gnu + run: sudo apt install -y crossbuild-essential-riscv64 - name: Install test tools packages if: env.AARCH64 != 'true' || env.PPC64LE != 'true' || env.RISCV64 != 'true' || env.RUN_TESTS == 'true' run: sudo apt install -y gdb -- 2.30.2
On Thu, Jun 9, 2022 at 2:17 PM Stanislaw Kardach <kda@semihalf.com> wrote:
>
> The current packages installed for RISC-V build check do not contain a
> C++ compiler, which hid an issue with C++ type conversion in the
> rte_vect.h header on RISC-V or in the scalar implementation of the LPM
> x4 lookup. Now that this issue is fixed, use the full toolchain install
> to enable the C++ test.
>
> Besides, the user's guide for RISC-V cross-compilation recommends the
> use of crossbuild-essential-riscv64.
>
> Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
On Thu, Jun 9, 2022 at 2:17 PM Stanislaw Kardach <kda@semihalf.com> wrote: > > rte_xmm_t is a union type which wraps around xmm_t and maps its contents > to scalar structures. Since C++ has stricter type conversion rules than > C, the rte_xmm_t::x has to be used instead of C-casting. > > The generated assembly is identical to the code without the fix (checked > both on x86 and RISC-V). Fixes: 406937f89ffd ("lpm: add scalar version of lookupx4") > > Signed-off-by: Stanislaw Kardach <kda@semihalf.com> Reviewed-by: David Marchand <david.marchand@redhat.com> > --- > lib/lpm/rte_lpm_scalar.h | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/lib/lpm/rte_lpm_scalar.h b/lib/lpm/rte_lpm_scalar.h > index f0d9f37894..161b40ff80 100644 > --- a/lib/lpm/rte_lpm_scalar.h > +++ b/lib/lpm/rte_lpm_scalar.h > @@ -15,18 +15,19 @@ extern "C" { > > static inline void > rte_lpm_lookupx4(const struct rte_lpm *lpm, xmm_t ip, uint32_t hop[4], > - uint32_t defv) > + uint32_t defv) Nit: unrelated whitespace change. Besides, indent is supposed to be one tab. This can be dropped when applying. -- David Marchand
On Thu, Jun 9, 2022 at 2:17 PM Stanislaw Kardach <kda@semihalf.com> wrote: > > rte_xmm_t is a union type which wraps around xmm_t and maps its contents > to scalar structures. Since C++ has stricter type conversion rules than > C, the rte_xmm_t::x has to be used instead of C-casting. > Fixes: f22e705ebf12 ("eal/riscv: support RISC-V architecture") > Signed-off-by: Stanislaw Kardach <kda@semihalf.com> Reviewed-by: David Marchand <david.marchand@redhat.com> -- David Marchand
Stanislaw Kardach <kda@semihalf.com> writes:
> The current packages installed for RISC-V build check do not contain a
> C++ compiler, which hid an issue with C++ type conversion in the
> rte_vect.h header on RISC-V or in the scalar implementation of the LPM
> x4 lookup. Now that this issue is fixed, use the full toolchain install
> to enable the C++ test.
>
> Besides, the user's guide for RISC-V cross-compilation recommends the
> use of crossbuild-essential-riscv64.
>
> Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
> ---
Acked-by: Aaron Conole <aconole@redhat.com>
On Thu, Jun 9, 2022 at 2:17 PM Stanislaw Kardach <kda@semihalf.com> wrote: > > As David noticed in [1] there is an issue with C++ compilation of the > rte_vect.h header in RISC-V. Upon closer inspection, the problem appears on > all architectures due to the type conversion rules in C++. > More precisely a union type rte_xmm_t requires a conversion constructor > from xmm_t type. > The most obvious fix is to use a structure initializer for such copies > (since rte_xmm_t union contains xmm_t anyway). The generated assembly > at -O2 is exactly the same, so there's no real impact. > > The bigger question is whether accessing bits of the architecture specific > xmm_t type in an array fashion is always correct? All current architectures > define rte_xmm_t in the same manner implying that. Copying other arch maintainers. > > Additionally change RISC-V CI settings to use crossbuild-essential-riscv64 > package which provides tools that enable C++ checks. > > [1] http://mails.dpdk.org/archives/dev/2022-June/243683.html > > Stanislaw Kardach (3): > eal/riscv: fix xmm_t casting for C++ > lpm: fix xmm_t casting for C++ in scalar version > ci: use crossbuild-essential-riscv64 for compiling In any case, this series looks good. Series applied, thanks Stanislaw. -- David Marchand
> On Thu, Jun 9, 2022 at 2:17 PM Stanislaw Kardach <kda@semihalf.com> wrote:
>>
>> As David noticed in [1] there is an issue with C++ compilation of the
>> rte_vect.h header in RISC-V. Upon closer inspection, the problem appears on
>> all architectures due to the type conversion rules in C++.
>> More precisely a union type rte_xmm_t requires a conversion constructor
>> from xmm_t type.
>> The most obvious fix is to use a structure initializer for such copies
>> (since rte_xmm_t union contains xmm_t anyway). The generated assembly
>> at -O2 is exactly the same, so there's no real impact.
>>
>> The bigger question is whether accessing bits of the architecture specific
>> xmm_t type in an array fashion is always correct? All current architectures
>> define rte_xmm_t in the same manner implying that.
>
> Copying other arch maintainers.
My read of the Altivec vector layout for LE systems says the existing
union operator rte_xmm_t is correct, though my C++ experience is
limited. How can I generate an error with C++ to expose this issue?
Dave
On Tue, Jun 21, 2022 at 12:54 AM David Christensen <drc@linux.vnet.ibm.com> wrote: > > > On Thu, Jun 9, 2022 at 2:17 PM Stanislaw Kardach <kda@semihalf.com> wrote: > >> > >> As David noticed in [1] there is an issue with C++ compilation of the > >> rte_vect.h header in RISC-V. Upon closer inspection, the problem appears on > >> all architectures due to the type conversion rules in C++. > >> More precisely a union type rte_xmm_t requires a conversion constructor > >> from xmm_t type. > >> The most obvious fix is to use a structure initializer for such copies > >> (since rte_xmm_t union contains xmm_t anyway). The generated assembly > >> at -O2 is exactly the same, so there's no real impact. > >> > >> The bigger question is whether accessing bits of the architecture specific > >> xmm_t type in an array fashion is always correct? All current architectures > >> define rte_xmm_t in the same manner implying that. > > > > Copying other arch maintainers. > > My read of the Altivec vector layout for LE systems says the existing > union operator rte_xmm_t is correct, though my C++ experience is > limited. How can I generate an error with C++ to expose this issue? > > Dave To replicate this issue: 1. Apply the patch below. In essence it forces the use of scalar lpm and changes C++ compiler to g++ so that meson properly detects it. Otherwise C++ checks won't be generated. 2. Configure build with: meson build-ppc64le --werror --cross-file config/ppc/ppc64le-power8-linux-gcc-ubuntu -Dcheck_includes=true 3. Build with: ninja -C build-ppc64le buildtools/chkincs/chkincs-cpp Note that the build target only gets generated if C++ compiler is properly discovered by meson. To be honest I'm not sure why powerpc64le-linux-gnu-cpp doesn't get properly picked up by meson. Also of interesting note the similar substitution should be made for arm64 but it seems like a separate thread. Patch for issue replication: diff --git a/config/arm/arm64_armv8_linux_gcc b/config/arm/arm64_armv8_linux_gcc index 5391d35389..5c32f6b9ca 100644 --- a/config/arm/arm64_armv8_linux_gcc +++ b/config/arm/arm64_armv8_linux_gcc @@ -1,6 +1,6 @@ [binaries] c = 'aarch64-linux-gnu-gcc' -cpp = 'aarch64-linux-gnu-cpp' +cpp = 'aarch64-linux-gnu-g++' ar = 'aarch64-linux-gnu-gcc-ar' strip = 'aarch64-linux-gnu-strip' pkgconfig = 'aarch64-linux-gnu-pkg-config' diff --git a/config/ppc/ppc64le-power8-linux-gcc-ubuntu b/config/ppc/ppc64le-power8-linux-gcc-ubuntu index 803c612cbc..2e6b13a406 100644 --- a/config/ppc/ppc64le-power8-linux-gcc-ubuntu +++ b/config/ppc/ppc64le-power8-linux-gcc-ubuntu @@ -1,6 +1,6 @@ [binaries] c = 'powerpc64le-linux-gnu-gcc' -cpp = 'powerpc64le-linux-gnu-cpp' +cpp = 'powerpc64le-linux-gnu-g++' ar = 'powerpc64le-linux-gnu-ar' strip = 'powerpc64le-linux-gnu-strip' diff --git a/lib/lpm/rte_lpm.h b/lib/lpm/rte_lpm.h index 4f38864fde..fd1362a027 100644 --- a/lib/lpm/rte_lpm.h +++ b/lib/lpm/rte_lpm.h @@ -397,19 +397,7 @@ static inline void rte_lpm_lookupx4(const struct rte_lpm *lpm, xmm_t ip, uint32_t hop[4], uint32_t defv); -#if defined(RTE_ARCH_ARM) -#ifdef RTE_HAS_SVE_ACLE -#include "rte_lpm_sve.h" -#else -#include "rte_lpm_neon.h" -#endif -#elif defined(RTE_ARCH_PPC_64) -#include "rte_lpm_altivec.h" -#elif defined(RTE_ARCH_X86) -#include "rte_lpm_sse.h" -#else #include "rte_lpm_scalar.h" -#endif #ifdef __cplusplus } -- Best Regards, Stanisław Kardach
On Tue, Jun 21, 2022 at 11:30:32AM +0200, Stanisław Kardach wrote:
> On Tue, Jun 21, 2022 at 12:54 AM David Christensen
> <drc@linux.vnet.ibm.com> wrote:
> >
> > > On Thu, Jun 9, 2022 at 2:17 PM Stanislaw Kardach <kda@semihalf.com> wrote:
> > >>
> > >> As David noticed in [1] there is an issue with C++ compilation of the
> > >> rte_vect.h header in RISC-V. Upon closer inspection, the problem appears on
> > >> all architectures due to the type conversion rules in C++.
> > >> More precisely a union type rte_xmm_t requires a conversion constructor
> > >> from xmm_t type.
> > >> The most obvious fix is to use a structure initializer for such copies
> > >> (since rte_xmm_t union contains xmm_t anyway). The generated assembly
> > >> at -O2 is exactly the same, so there's no real impact.
> > >>
> > >> The bigger question is whether accessing bits of the architecture specific
> > >> xmm_t type in an array fashion is always correct? All current architectures
> > >> define rte_xmm_t in the same manner implying that.
> > >
> > > Copying other arch maintainers.
> >
> > My read of the Altivec vector layout for LE systems says the existing
> > union operator rte_xmm_t is correct, though my C++ experience is
> > limited. How can I generate an error with C++ to expose this issue?
> >
> > Dave
> To replicate this issue:
> 1. Apply the patch below. In essence it forces the use of scalar lpm
> and changes C++ compiler to g++ so that meson properly detects it.
> Otherwise C++ checks won't be generated.
> 2. Configure build with: meson build-ppc64le --werror --cross-file
> config/ppc/ppc64le-power8-linux-gcc-ubuntu -Dcheck_includes=true
> 3. Build with: ninja -C build-ppc64le buildtools/chkincs/chkincs-cpp
>
> Note that the build target only gets generated if C++ compiler is
> properly discovered by meson. To be honest I'm not sure why
> powerpc64le-linux-gnu-cpp doesn't get properly picked up by meson.
Generally the "cpp" binary is not the c-plus-plus one, but the C
preprocessor one. Perhaps the original files are incorrect here, and should
all refer to g++.
/Bruce
On Tue, Jun 21, 2022 at 11:39 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> <snip>
> Generally the "cpp" binary is not the c-plus-plus one, but the C
> preprocessor one. Perhaps the original files are incorrect here, and should
> all refer to g++.
>
> /Bruce
>
That does make sense. I'll submit a separate patchset fixing all
occurrences (of which there are many).
--
Best Regards,
Stanisław Kardach
On Tue, Jun 21, 2022 at 11:42:55AM +0200, Stanisław Kardach wrote: > On Tue, Jun 21, 2022 at 11:39 AM Bruce Richardson > <bruce.richardson@intel.com> wrote: > > <snip> > > Generally the "cpp" binary is not the c-plus-plus one, but the C > > preprocessor one. Perhaps the original files are incorrect here, and should > > all refer to g++. > > > > /Bruce > > > That does make sense. I'll submit a separate patchset fixing all > occurrences (of which there are many). > As a more general note for future consideration, I notice that in meson 0.56 the cross-file support has been enhanced with the ability to use constants and therefore separate out prefixes.[1] When we get to the point where we feel we can mandate meson 0.56 upwards for cross compilation, we should look to leverage this. It should even allow other scripts such as test-meson-builds to auto-generate the constant paths to the binaries on the fly, effectively allowing the use of environment variables for these - something previously requested by Thomas. /Bruce [1] https://mesonbuild.com/Machine-files.html#constants
21/06/2022 11:49, Bruce Richardson: > On Tue, Jun 21, 2022 at 11:42:55AM +0200, Stanisław Kardach wrote: > > On Tue, Jun 21, 2022 at 11:39 AM Bruce Richardson > > <bruce.richardson@intel.com> wrote: > > > <snip> > > > Generally the "cpp" binary is not the c-plus-plus one, but the C > > > preprocessor one. Perhaps the original files are incorrect here, and should > > > all refer to g++. > > > > > > /Bruce > > > > > That does make sense. I'll submit a separate patchset fixing all > > occurrences (of which there are many). > > > > As a more general note for future consideration, I notice that in meson > 0.56 the cross-file support has been enhanced with the ability to use > constants and therefore separate out prefixes.[1] > > When we get to the point where we feel we can mandate meson 0.56 upwards > for cross compilation, we should look to leverage this. It should even > allow other scripts such as test-meson-builds to auto-generate the constant > paths to the binaries on the fly, effectively allowing the use of > environment variables for these - something previously requested by Thomas. That would be great. Cross compilation prefix is such a basic thing, we should handle it properly. > [1] https://mesonbuild.com/Machine-files.html#constants
On Tue, Jun 21, 2022 at 12:22 PM Thomas Monjalon <thomas@monjalon.net> wrote: > > 21/06/2022 11:49, Bruce Richardson: > > On Tue, Jun 21, 2022 at 11:42:55AM +0200, Stanisław Kardach wrote: > > > On Tue, Jun 21, 2022 at 11:39 AM Bruce Richardson > > > <bruce.richardson@intel.com> wrote: > > > > <snip> > > > > Generally the "cpp" binary is not the c-plus-plus one, but the C > > > > preprocessor one. Perhaps the original files are incorrect here, and should > > > > all refer to g++. > > > > > > > > /Bruce > > > > > > > That does make sense. I'll submit a separate patchset fixing all > > > occurrences (of which there are many). > > > > > > > As a more general note for future consideration, I notice that in meson > > 0.56 the cross-file support has been enhanced with the ability to use > > constants and therefore separate out prefixes.[1] > > > > When we get to the point where we feel we can mandate meson 0.56 upwards > > for cross compilation, we should look to leverage this. It should even > > allow other scripts such as test-meson-builds to auto-generate the constant > > paths to the binaries on the fly, effectively allowing the use of > > environment variables for these - something previously requested by Thomas. > > That would be great. > Cross compilation prefix is such a basic thing, we should handle it properly. Please correct me if I'm wrong but it seems that meson's approach to cross-compiling is to package all settings into cross-files. Probably under assumption that a repeatable compilation is more important than flexibility and that there are compiler-specific knobs that need/can to be tuned. Therefore reading CROSS_COMPILE/prefix from environment is intentionally made hard. So should the direction be environment or rather separating cross-files into arch-part and toolchain-parts and letting user create his own toolchain part while maintaining a matrix of supported combinations for CI? I'm not advocating either, just want to wrap my head around it. > > > [1] https://mesonbuild.com/Machine-files.html#constants > > >
21/06/2022 13:05, Stanisław Kardach: > On Tue, Jun 21, 2022 at 12:22 PM Thomas Monjalon <thomas@monjalon.net> wrote: > > > > 21/06/2022 11:49, Bruce Richardson: > > > On Tue, Jun 21, 2022 at 11:42:55AM +0200, Stanisław Kardach wrote: > > > > On Tue, Jun 21, 2022 at 11:39 AM Bruce Richardson > > > > <bruce.richardson@intel.com> wrote: > > > > > <snip> > > > > > Generally the "cpp" binary is not the c-plus-plus one, but the C > > > > > preprocessor one. Perhaps the original files are incorrect here, and should > > > > > all refer to g++. > > > > > > > > > > /Bruce > > > > > > > > > That does make sense. I'll submit a separate patchset fixing all > > > > occurrences (of which there are many). > > > > > > > > > > As a more general note for future consideration, I notice that in meson > > > 0.56 the cross-file support has been enhanced with the ability to use > > > constants and therefore separate out prefixes.[1] > > > > > > When we get to the point where we feel we can mandate meson 0.56 upwards > > > for cross compilation, we should look to leverage this. It should even > > > allow other scripts such as test-meson-builds to auto-generate the constant > > > paths to the binaries on the fly, effectively allowing the use of > > > environment variables for these - something previously requested by Thomas. > > > > That would be great. > > Cross compilation prefix is such a basic thing, we should handle it properly. > Please correct me if I'm wrong but it seems that meson's approach to > cross-compiling is to package all settings into cross-files. Probably > under assumption that a repeatable compilation is more important than > flexibility and that there are compiler-specific knobs that need/can > to be tuned. Therefore reading CROSS_COMPILE/prefix from environment > is intentionally made hard. If it is made intentionally hard, it is just a wrong design. A toolchain prefix is just a name. We can have 2 toolchains compiled with the same name and different behaviours. And we can have 2 similar toolchains with a different name. > So should the direction be environment or rather separating > cross-files into arch-part and toolchain-parts and letting user create > his own toolchain part while maintaining a matrix of supported > combinations for CI? I'm not advocating either, just want to wrap my > head around it. We should be able to use a toolchain compiled anywhere without modifying the cross files, just because a "-gnu-" is missing or any other irrelevant detail.
On Tue, Jun 21, 2022 at 1:53 PM Thomas Monjalon <thomas@monjalon.net> wrote: > <snip> > If it is made intentionally hard, it is just a wrong design. > A toolchain prefix is just a name. > We can have 2 toolchains compiled with the same name and different behaviours. > And we can have 2 similar toolchains with a different name. I don't think meson will allow it anytime soon (see [1]). The reasoning being that it's easy to screw up the environment and not notice it where as files are persistent. > > > So should the direction be environment or rather separating > > cross-files into arch-part and toolchain-parts and letting user create > > his own toolchain part while maintaining a matrix of supported > > combinations for CI? I'm not advocating either, just want to wrap my > > head around it. > > We should be able to use a toolchain compiled anywhere > without modifying the cross files, just because a "-gnu-" is missing > or any other irrelevant detail. I've checked that if I remove the binaries from a cross-file, then specifying CC/CXX/AR/STRIP environment variables is picked up by meson: AR=riscv64-unknown-linux-gnu-ar \ STRIP=riscv64-unknown-linux-gnu-strip \ CC=riscv64-unknown-linux-gnu-gcc \ CXX=riscv64-unknown-linux-gnu-g++ \ meson build-rv-test --cross-file=config/riscv/riscv64_linux_gcc But then there are no default values. A suggested frankenstein-like solution in [1] is to use a script that generates a cross-file with [constants] section and launches meson with it. [1] https://github.com/mesonbuild/meson/issues/9#issuecomment-381410972
On Tue, Jun 21, 2022 at 02:37:51PM +0200, Stanisław Kardach wrote:
> On Tue, Jun 21, 2022 at 1:53 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > <snip>
> > If it is made intentionally hard, it is just a wrong design.
> > A toolchain prefix is just a name.
> > We can have 2 toolchains compiled with the same name and different behaviours.
> > And we can have 2 similar toolchains with a different name.
> I don't think meson will allow it anytime soon (see [1]). The
> reasoning being that it's easy to screw up the environment and not
> notice it where as files are persistent.
> >
> > > So should the direction be environment or rather separating
> > > cross-files into arch-part and toolchain-parts and letting user create
> > > his own toolchain part while maintaining a matrix of supported
> > > combinations for CI? I'm not advocating either, just want to wrap my
> > > head around it.
> >
> > We should be able to use a toolchain compiled anywhere
> > without modifying the cross files, just because a "-gnu-" is missing
> > or any other irrelevant detail.
> I've checked that if I remove the binaries from a cross-file, then
> specifying CC/CXX/AR/STRIP environment variables is picked up by
> meson:
> AR=riscv64-unknown-linux-gnu-ar \
> STRIP=riscv64-unknown-linux-gnu-strip \
> CC=riscv64-unknown-linux-gnu-gcc \
> CXX=riscv64-unknown-linux-gnu-g++ \
> meson build-rv-test --cross-file=config/riscv/riscv64_linux_gcc
> But then there are no default values.
>
> A suggested frankenstein-like solution in [1] is to use a script that
> generates a cross-file with [constants] section and launches meson
> with it.
>
> [1] https://github.com/mesonbuild/meson/issues/9#issuecomment-381410972
This is all incidental to the original fix, which is to replace cpp with
g++ in the existing cross files. Any update to how cross files are used by
our DPDK scripts, is a different discussion for a different day!
[Sorry to have brought it up here, it just seemed somewhat relevant at the
time!]
On Tue, Jun 21, 2022 at 11:42:55AM +0200, Stanisław Kardach wrote: > On Tue, Jun 21, 2022 at 11:39 AM Bruce Richardson > <bruce.richardson@intel.com> wrote: > > <snip> > > Generally the "cpp" binary is not the c-plus-plus one, but the C > > preprocessor one. Perhaps the original files are incorrect here, and should > > all refer to g++. > > > > /Bruce agreed cpp is the preprocessor and one of 2 conventions for file extension of c++ files (the other being .cc) conventionally you're looking for CXX i.e. as in env CXX=g++ > > > That does make sense. I'll submit a separate patchset fixing all > occurrences (of which there are many). > > -- > Best Regards, > Stanisław Kardach