Re: [PATCH v2 2/3] target/arm/tcg/m_helper.c: Include the full helpers only with CONFIG_ARM_V7M

[Thomas Huth]

On 01/02/2024 15.19, Peter Maydell wrote:
> On Mon, 29 Jan 2024 at 08:18, Thomas Huth <thuth@redhat.com> wrote:
> > If CONFIG_ARM_V7M is not set, we don't want to include the full-fledged
> > helper functions that require additional functions for linking. The
> > reduced set of the linux-user functions works fine as stubs in this
> > case, so change the #ifdef statement accordingly.
> >
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   target/arm/tcg/m_helper.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/arm/tcg/m_helper.c b/target/arm/tcg/m_helper.c
> > index d1f1e02acc..a5a6e96fc3 100644
> > --- a/target/arm/tcg/m_helper.c
> > +++ b/target/arm/tcg/m_helper.c
> > @@ -22,6 +22,7 @@
> >   #endif
> >   #if !defined(CONFIG_USER_ONLY)
> >   #include "hw/intc/armv7m_nvic.h"
> > +#include CONFIG_DEVICES
> >   #endif
> >
> >   static void v7m_msr_xpsr(CPUARMState *env, uint32_t mask,
> > @@ -69,7 +70,7 @@ uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t 
> > secure)
> >       return value;
> >   }
> >
> > -#ifdef CONFIG_USER_ONLY
> > +#if defined(CONFIG_USER_ONLY) || !defined(CONFIG_ARM_V7M)
>
> This looks a bit odd. If we don't have CONFIG_ARM_V7M
> why are we compiling this file at all?

We'll get failures during linking otherwise. target/arm/helper.h still 
defines code that requires the v7m_* helper functions:

/usr/bin/ld: libqemu-arm-softmmu.fa.p/target_arm_tcg_translate.c.o: 
qemu/target/arm/helper.h:76: undefined reference to `helper_v7m_vlldm'
/usr/bin/ld: libqemu-arm-softmmu.fa.p/target_arm_tcg_translate.c.o: 
qemu/target/arm/helper.h:75: undefined reference to `helper_v7m_vlstm'
/usr/bin/ld: libqemu-arm-softmmu.fa.p/target_arm_tcg_translate.c.o: 
qemu/target/arm/helper.h:73: undefined reference to 
`helper_v7m_preserve_fp_state'

etc.

And then other files are depending on these helpers, too, e.g. 
target/arm/tcg/translate-vfp.c calls gen_helper_v7m_preserve_fp_state() etc. 
... I guess it might be possible to disable all those spots with some 
#ifdefs, too, but it's getting quite ugly that way. It's way nicer to use 
the stub functions that we have in m_helper already anyway.

 Thomas