Re: [Buildroot] [PATCH v3 4/4] package/fluent-bit: rework wasm handling

From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Thomas Devoogdt <thomas@devoogdt.com>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH v3 4/4] package/fluent-bit: rework wasm handling
Date: Mon, 24 Jul 2023 10:45:58 +0200	[thread overview]
Message-ID: <20230724104558.43d489a5@windsurf> (raw)
In-Reply-To: <CACXRmJiF+S5C4-4VFK68jBi4g5On1FBO3js9+cvxAwF_NxzZRw@mail.gmail.com>

Hello Thomas,

On Mon, 24 Jul 2023 10:29:45 +0200
Thomas Devoogdt <thomas@devoogdt.com> wrote:

> > Something that isn't explained in your commit log is why you're making
> > the wasm feature automatically enabled, rather than optional as it was?  
> 
> I had to re-add the logic to the make file in order to select the right target.
> I dropped the Config.In logic to avoid double maintenance of the
> supported architectures.

Well, in fact the logic for the architecture would be cleaner in the
Config.in file.

if BR2_PACKAGE_FLUENT_BIT

config BR2_PACKAGE_FLUENT_BIT_WASM_ARCH
	string
	default "AARCH64" 	if BR2_aarch64 || BR2_aarch64be
	default "ARC" 		if BR2_arc || BR2_arceb
	default "ARM" 		if BR2_arm || BR2_armeb
	...

config BR2_PACKAGE_FLUENT_BIT_WASM_ARCH_SUPPORTS
	bool
	default y if BR2_PACKAGE_FLUENT_BIT_WASM_ARCH != ""

config BR2_PACKAGE_FLUENT_BIT_WASM
	bool "wasm runtime support"
	depends on BR2_INSTALL_LIBSTDCPP
	depends on BR2_PACKAGE_FLUENT_BIT_WASM_ARCH_SUPPORTS
	help
	  Support for WASM-based plugins.

...

endif

And then in the Makefile:

FLUENT_BIT_WASM_ARCH = $(call qstrip,$(BR2_PACKAGE_FLUENT_BIT_WASM_ARCH))

and voilà.

> > What is the size impact? Would someone possibly want fluent-bit without
> > wasm support?  
> 
> FLB_WASM=No : -rwxr-xr-x 1 thomas thomas 31769156 Jul 24 09:10
> output/target/usr/lib/fluent-bit/libfluent-bit.so
> FLB_WASM=Yes: -rwxr-xr-x 1 thomas thomas 33801056 Jul 24 09:07
> output/target/usr/lib/fluent-bit/libfluent-bit.so
> 
> So the binary goes from 31.8MB to 33.8MB.

Admittedly, it's already a huge thing, so another 2 MB more...

But in general in Buildroot our policy is to ensure things that are
optional in upstream are indeed optional in Buildroot.

> package/fluent-bit: rework wasm handling
> 
> Fluent-bit isn't very great at supporting the various architectures,
> so help a little bit by defining the WAMR_BUILD_TARGET ourselves.
> 
> The BR2_PACKAGE_FLUENT_BIT_WASM option is removed to avoid
> double architecture maintenance in both the KConfig and make file. This

See above: no double maintenance is needed.

> means that WAMR will always get compiled resulting in a slightly binary
> size increase from 31.8MB to 33.8MB.
> 
> Two failures while executing ./utils/test-pkg:
> 
> - sourcery-mips64:
> core/iwasm/common/arch/invokeNative_mips.s: Assembler messages:
> core/iwasm/common/arch/invokeNative_mips.s:58: Error: opcode not
> supported on this processor: mips64r2 (mips64r2) `ldc1 $f12,0($sp)'
> core/iwasm/common/arch/invokeNative_mips.s:59: Error: opcode not
> supported on this processor: mips64r2 (mips64r2) `ldc1 $f14,8($sp)'
> make[4]: *** [src/wasm/CMakeFiles/vmlib-static.dir/build.make:495:
> src/wasm/CMakeFiles/vmlib-static.dir/__/__/lib/wasm-micro-runtime-WAMR-1.1.1/core/iwasm/common/arch/invokeNative_mips.s.o]
> Error 1
> make[4]: *** Waiting for unfinished jobs....
> 
> - sourcery-mips:
> core/iwasm/common/arch/invokeNative_mips.s: Assembler messages:
> core/iwasm/common/arch/invokeNative_mips.s:58: Error: opcode not
> supported on this processor: mips32r2 (mips32r2) `ldc1 $f12,0($sp)'
> core/iwasm/common/arch/invokeNative_mips.s:59: Error: opcode not
> supported on this processor: mips32r2 (mips32r2) `ldc1 $f14,8($sp)'
> make[4]: *** [src/wasm/CMakeFiles/vmlib-static.dir/build.make:495:
> src/wasm/CMakeFiles/vmlib-static.dir/__/__/lib/wasm-micro-runtime-WAMR-1.1.1/core/iwasm/common/arch/invokeNative_mips.s.o]
> Error 1
> make[4]: *** Waiting for unfinished jobs....
> 
> Which are fixed by setting the obscure flag "WAMR_BUILD_INVOKE_NATIVE_GENERAL".
> 
> See https://github.com/bytecodealliance/wasm-micro-runtime/issues/625
> for more info:
> “Hi, building the invokeNative_general.c explicitly is supported now,
> please pull the latest code and use
> "cmake -DWAMR_BUILD_INVOKE_NATIVE_GENERAL=1 ..." to build it.”

Thanks, this explanation looks good.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot