This code is only relevant when TCG is present in the build. Building
with --disable-tcg --enable-xen on an x86 host we get:
$ ../configure --target-list=x86_64-softmmu,aarch64-softmmu --disable-tcg --enable-xen
$ make -j$(nproc)
...
libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `m_sysreg_ptr':
../target/arm/gdbstub.c:358: undefined reference to `arm_v7m_get_sp_ptr'
../target/arm/gdbstub.c:361: undefined reference to `arm_v7m_get_sp_ptr'
libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `arm_gdb_get_m_systemreg':
../target/arm/gdbstub.c:405: undefined reference to `arm_v7m_mrs_control'
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
This is a respin of:
https://lore.kernel.org/r/20230313151058.19645-5-farosas@suse.de
---
target/arm/gdbstub.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 03b17c814f..f421c5d041 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -324,6 +324,7 @@ static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
return cpu->dyn_sysreg_xml.num;
}
+#ifdef CONFIG_TCG
typedef enum {
M_SYSREG_MSP,
M_SYSREG_PSP,
@@ -481,6 +482,7 @@ static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
return cpu->dyn_m_secextreg_xml.num;
}
#endif
+#endif /* CONFIG_TCG */
const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
{
@@ -561,6 +563,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
"system-registers.xml", 0);
+#ifdef CONFIG_TCG
if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) {
gdb_register_coprocessor(cs,
arm_gdb_get_m_systemreg, arm_gdb_set_m_systemreg,
@@ -575,4 +578,5 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
}
#endif
}
+#endif /* CONFIG_TCG */
}
--
2.35.3On 28/6/23 18:48, Fabiano Rosas wrote:
> This code is only relevant when TCG is present in the build. Building
> with --disable-tcg --enable-xen on an x86 host we get:
>
> $ ../configure --target-list=x86_64-softmmu,aarch64-softmmu --disable-tcg --enable-xen
> $ make -j$(nproc)
> ...
> libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `m_sysreg_ptr':
> ../target/arm/gdbstub.c:358: undefined reference to `arm_v7m_get_sp_ptr'
> ../target/arm/gdbstub.c:361: undefined reference to `arm_v7m_get_sp_ptr'
>
> libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `arm_gdb_get_m_systemreg':
> ../target/arm/gdbstub.c:405: undefined reference to `arm_v7m_mrs_control'
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> This is a respin of:
> https://lore.kernel.org/r/20230313151058.19645-5-farosas@suse.de
> ---
> target/arm/gdbstub.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index 03b17c814f..f421c5d041 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -324,6 +324,7 @@ static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
> return cpu->dyn_sysreg_xml.num;
> }
>
> +#ifdef CONFIG_TCG
OK.
> typedef enum {
> M_SYSREG_MSP,
> M_SYSREG_PSP,
> @@ -481,6 +482,7 @@ static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
> return cpu->dyn_m_secextreg_xml.num;
> }
> #endif
> +#endif /* CONFIG_TCG */
>
> const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
> {
> @@ -561,6 +563,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
> arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
> "system-registers.xml", 0);
>
> +#ifdef CONFIG_TCG
IIUC tcg_enabled(), this guard shouldn't be necessary; if CONFIG_TCG
is not defined, tcg_enabled() evaluates to 0, and the compiler should
elide the whole block.
> if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) {
> gdb_register_coprocessor(cs,
> arm_gdb_get_m_systemreg, arm_gdb_set_m_systemreg,
> @@ -575,4 +578,5 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
> }
> #endif
> }
> +#endif /* CONFIG_TCG */
> }
On Tue, 4 Jul 2023 at 16:21, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 28/6/23 18:48, Fabiano Rosas wrote:
> > This code is only relevant when TCG is present in the build. Building
> > with --disable-tcg --enable-xen on an x86 host we get:
> >
> > $ ../configure --target-list=x86_64-softmmu,aarch64-softmmu --disable-tcg --enable-xen
> > $ make -j$(nproc)
> > ...
> > libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `m_sysreg_ptr':
> > ../target/arm/gdbstub.c:358: undefined reference to `arm_v7m_get_sp_ptr'
> > ../target/arm/gdbstub.c:361: undefined reference to `arm_v7m_get_sp_ptr'
> >
> > libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `arm_gdb_get_m_systemreg':
> > ../target/arm/gdbstub.c:405: undefined reference to `arm_v7m_mrs_control'
> >
> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
> > ---
> > This is a respin of:
> > https://lore.kernel.org/r/20230313151058.19645-5-farosas@suse.de
> > ---
> > target/arm/gdbstub.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> > index 03b17c814f..f421c5d041 100644
> > --- a/target/arm/gdbstub.c
> > +++ b/target/arm/gdbstub.c
> > @@ -324,6 +324,7 @@ static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
> > return cpu->dyn_sysreg_xml.num;
> > }
> >
> > +#ifdef CONFIG_TCG
>
> OK.
>
> > typedef enum {
> > M_SYSREG_MSP,
> > M_SYSREG_PSP,
> > @@ -481,6 +482,7 @@ static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
> > return cpu->dyn_m_secextreg_xml.num;
> > }
> > #endif
> > +#endif /* CONFIG_TCG */
> >
> > const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
> > {
> > @@ -561,6 +563,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
> > arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
> > "system-registers.xml", 0);
> >
> > +#ifdef CONFIG_TCG
>
> IIUC tcg_enabled(), this guard shouldn't be necessary; if CONFIG_TCG
> is not defined, tcg_enabled() evaluates to 0, and the compiler should
> elide the whole block.
IME it's a bit optimistic to assume that the compiler will always
do that, especially with no optimisation enabled.
thanks
-- PMM
On 7/4/23 17:44, Peter Maydell wrote:
>> IIUC tcg_enabled(), this guard shouldn't be necessary; if CONFIG_TCG
>> is not defined, tcg_enabled() evaluates to 0, and the compiler should
>> elide the whole block.
>
> IME it's a bit optimistic to assume that the compiler will always
> do that, especially with no optimisation enabled.
There's plenty of other places that we do.
The compiler is usually pretty good with "if (0)".
My question is if
> if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) {
needs to be written
if (tcg_enabled()) {
if (arm_feature(..., M) {
...
}
}
r~
Richard Henderson <richard.henderson@linaro.org> writes:
> On 7/4/23 17:44, Peter Maydell wrote:
>>> IIUC tcg_enabled(), this guard shouldn't be necessary; if CONFIG_TCG
>>> is not defined, tcg_enabled() evaluates to 0, and the compiler should
>>> elide the whole block.
>>
>> IME it's a bit optimistic to assume that the compiler will always
>> do that, especially with no optimisation enabled.
>
> There's plenty of other places that we do.
> The compiler is usually pretty good with "if (0)".
>
> My question is if
>
>> if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) {
>
> needs to be written
>
> if (tcg_enabled()) {
> if (arm_feature(..., M) {
> ...
> }
> }
Yeah, that doesn't work either. I don't understand why in this
particular case the compiler seems unable to remove that code.
Can anyone else reproduce this or is it just happening on my setup?
Maybe something is broken on my side...
On 4/7/23 17:44, Peter Maydell wrote:
> On Tue, 4 Jul 2023 at 16:21, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 28/6/23 18:48, Fabiano Rosas wrote:
>>> This code is only relevant when TCG is present in the build. Building
>>> with --disable-tcg --enable-xen on an x86 host we get:
>>>
>>> $ ../configure --target-list=x86_64-softmmu,aarch64-softmmu --disable-tcg --enable-xen
>>> $ make -j$(nproc)
>>> ...
>>> libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `m_sysreg_ptr':
>>> ../target/arm/gdbstub.c:358: undefined reference to `arm_v7m_get_sp_ptr'
>>> ../target/arm/gdbstub.c:361: undefined reference to `arm_v7m_get_sp_ptr'
>>>
>>> libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `arm_gdb_get_m_systemreg':
>>> ../target/arm/gdbstub.c:405: undefined reference to `arm_v7m_mrs_control'
>>>
>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>> ---
>>> This is a respin of:
>>> https://lore.kernel.org/r/20230313151058.19645-5-farosas@suse.de
>>> ---
>>> target/arm/gdbstub.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
>>> index 03b17c814f..f421c5d041 100644
>>> --- a/target/arm/gdbstub.c
>>> +++ b/target/arm/gdbstub.c
>>> @@ -324,6 +324,7 @@ static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
>>> return cpu->dyn_sysreg_xml.num;
>>> }
>>>
>>> +#ifdef CONFIG_TCG
>>
>> OK.
>>
>>> typedef enum {
>>> M_SYSREG_MSP,
>>> M_SYSREG_PSP,
>>> @@ -481,6 +482,7 @@ static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
>>> return cpu->dyn_m_secextreg_xml.num;
>>> }
>>> #endif
>>> +#endif /* CONFIG_TCG */
>>>
>>> const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
>>> {
>>> @@ -561,6 +563,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
>>> arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
>>> "system-registers.xml", 0);
>>>
>>> +#ifdef CONFIG_TCG
>>
>> IIUC tcg_enabled(), this guard shouldn't be necessary; if CONFIG_TCG
>> is not defined, tcg_enabled() evaluates to 0, and the compiler should
>> elide the whole block.
>
> IME it's a bit optimistic to assume that the compiler will always
> do that, especially with no optimisation enabled.
OK I see, thanks.
On Wed, 28 Jun 2023 at 17:48, Fabiano Rosas <farosas@suse.de> wrote: > > This code is only relevant when TCG is present in the build. Building > with --disable-tcg --enable-xen on an x86 host we get: > > $ ../configure --target-list=x86_64-softmmu,aarch64-softmmu --disable-tcg --enable-xen > $ make -j$(nproc) > ... > libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `m_sysreg_ptr': > ../target/arm/gdbstub.c:358: undefined reference to `arm_v7m_get_sp_ptr' > ../target/arm/gdbstub.c:361: undefined reference to `arm_v7m_get_sp_ptr' > > libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `arm_gdb_get_m_systemreg': > ../target/arm/gdbstub.c:405: undefined reference to `arm_v7m_mrs_control' > > Signed-off-by: Fabiano Rosas <farosas@suse.de> Applied to target-arm.next, thanks. -- PMM
On 28/6/23 18:48, Fabiano Rosas wrote: > This code is only relevant when TCG is present in the build. Building > with --disable-tcg --enable-xen on an x86 host we get: > > $ ../configure --target-list=x86_64-softmmu,aarch64-softmmu --disable-tcg --enable-xen > $ make -j$(nproc) > ... > libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `m_sysreg_ptr': > ../target/arm/gdbstub.c:358: undefined reference to `arm_v7m_get_sp_ptr' > ../target/arm/gdbstub.c:361: undefined reference to `arm_v7m_get_sp_ptr' > > libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `arm_gdb_get_m_systemreg': > ../target/arm/gdbstub.c:405: undefined reference to `arm_v7m_mrs_control' I'm a bit confused, isn't this covered by the cross-arm64-xen-only job? > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > This is a respin of: > https://lore.kernel.org/r/20230313151058.19645-5-farosas@suse.de > --- > target/arm/gdbstub.c | 4 ++++ > 1 file changed, 4 insertions(+)
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 28/6/23 18:48, Fabiano Rosas wrote: >> This code is only relevant when TCG is present in the build. Building >> with --disable-tcg --enable-xen on an x86 host we get: >> >> $ ../configure --target-list=x86_64-softmmu,aarch64-softmmu --disable-tcg --enable-xen >> $ make -j$(nproc) >> ... >> libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `m_sysreg_ptr': >> ../target/arm/gdbstub.c:358: undefined reference to `arm_v7m_get_sp_ptr' >> ../target/arm/gdbstub.c:361: undefined reference to `arm_v7m_get_sp_ptr' >> >> libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `arm_gdb_get_m_systemreg': >> ../target/arm/gdbstub.c:405: undefined reference to `arm_v7m_mrs_control' > > I'm a bit confused, isn't this covered by the cross-arm64-xen-only > job? It should be. Perhaps the CI is using different optimization flags. I'll try to figure it out.
Fabiano Rosas <farosas@suse.de> writes: > Philippe Mathieu-Daudé <philmd@linaro.org> writes: > >> On 28/6/23 18:48, Fabiano Rosas wrote: >>> This code is only relevant when TCG is present in the build. Building >>> with --disable-tcg --enable-xen on an x86 host we get: >>> >>> $ ../configure --target-list=x86_64-softmmu,aarch64-softmmu --disable-tcg --enable-xen >>> $ make -j$(nproc) >>> ... >>> libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `m_sysreg_ptr': >>> ../target/arm/gdbstub.c:358: undefined reference to `arm_v7m_get_sp_ptr' >>> ../target/arm/gdbstub.c:361: undefined reference to `arm_v7m_get_sp_ptr' >>> >>> libqemu-aarch64-softmmu.fa.p/target_arm_gdbstub.c.o: in function `arm_gdb_get_m_systemreg': >>> ../target/arm/gdbstub.c:405: undefined reference to `arm_v7m_mrs_control' >> >> I'm a bit confused, isn't this covered by the cross-arm64-xen-only >> job? > > It should be. Perhaps the CI is using different optimization flags. I'll > try to figure it out. Yep. The CI has -O2 while I am using --enable-debug.
© 2016 - 2026 Red Hat, Inc.