Re: [RFC PATCH v1 01/43] Add option to enable/disable helper-to-tcg

[Anton Johansson]

On 22/11/24, Paolo Bonzini wrote:
> On 11/22/24 18:30, Richard Henderson wrote:
> > On 11/20/24 19:49, Anton Johansson wrote:
> > > Adds a meson option for enabling/disabling helper-to-tcg along with a
> > > CONFIG_* definition.
> > > 
> > > CONFIG_* will in future commits be used to conditionally include the
> > > helper-to-tcg subproject, and to remove unneeded code/memory when
> > > helper-to-tcg is not in use.
> > > 
> > > Current meson option is limited to Hexagon, as helper-to-tcg will be
> > > included as a subproject from target/hexagon.  This will change in the
> > > future if multiple frontends adopt helper-to-tcg.
> > > 
> > > Signed-off-by: Anton Johansson <anjo@rev.ng>
> > > ---
> > >   meson.build                   | 7 +++++++
> > >   meson_options.txt             | 2 ++
> > >   scripts/meson-buildoptions.sh | 5 +++++
> > >   3 files changed, 14 insertions(+)
> > 
> > Looks ok.  Could probably stand another set of meson eyes.
> > 
> > Acked-by: Richard Henderson <richard.henderson@linaro.org>
> 
> /me bows
> 
> Since the subproject has a pretty hefty (and specific) set of
> dependencies, please make this a "feature" option.  This allows
> subprojects/helper-to-tcg to disable itself if it cannot find
> a dependency or otherwise invokes error(), without breaking the
> build.  The --enable-hexagon-helper-to-tcg flag however *will*
> force the subproject to be buildable, just like all other
> QEMU feature options.
> 
> Something like this:
> 
> 
> ########################
> # Target configuration #
> ########################
> 
> # a bit gross to hardcode hexagon, but acceptable given the name of the option
> helper_to_tcg = subproject('helper-to-tcg', 
> get_option('hexagon_helper_to_tcg') \
>    .disable_auto_if('hexagon-linux-user' not in target_dirs))
> 
> 
> and replace helper_to_tcg_enabled throughout with helper_to_tcg.found().
> 
> > > +  if helper_to_tcg_enabled
> > > +    config_target += {
> > > +      'CONFIG_HELPER_TO_TCG': 'y',
> > > +    }
> > > +  endif
> 
> Here I would add instead add CONFIG_HELPER_TO_TCG (maybe renamed to
> TARGET_HELPER_TO_TCG) in configs/targets/) and add before the loop:
> 
> ignored = [ 'TARGET_XML_FILES', 'TARGET_ABI_DIR', 'TARGET_ARCH' ]
> if not helper_to_tcg.found()
>   # do not define it if it is not usable
>   ignored += ['TARGET_HELPER_TO_TCG']
> endif
> 
> Paolo
> 

Makes sense, appreciate it!:) There's always something new to learn about meson

//Anton