| Message ID | 20210420103616.32731-5-cfontana@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | s390x cleanup | expand |
On 20.04.21 12:36, Claudio Fontana wrote: > now that we protect all calls to the tcg-specific functions > with if (tcg_enabled()), we do not need the TCG stub anymore. You need compile-time checks, not runtime checks. Any calls have to be protected by #ifdef, otherwise the compiler might bail out. Maybe you just wanted to state it differently in this patch description.
On 4/20/21 2:54 PM, David Hildenbrand wrote: > On 20.04.21 12:36, Claudio Fontana wrote: >> now that we protect all calls to the tcg-specific functions >> with if (tcg_enabled()), we do not need the TCG stub anymore. > > You need compile-time checks, not runtime checks. Any calls have to be > protected by #ifdef, otherwise the compiler might bail out. This is not true though, tcg_enabled() is #defined as 0 if tcg is not enabled. #define kvm_enabled() (0) Compiler will elide the code if after the preprocessor pass the code is: if (0) { } It adds the benefit of actually checking the syntax of the code inside. As long as the prototypes are in sight, we rely on this for i386 and ARM already, to avoid accumulating stubs. > > Maybe you just wanted to state it differently in this patch description. > Thanks, Claudio
On 20.04.21 15:00, Claudio Fontana wrote: > On 4/20/21 2:54 PM, David Hildenbrand wrote: >> On 20.04.21 12:36, Claudio Fontana wrote: >>> now that we protect all calls to the tcg-specific functions >>> with if (tcg_enabled()), we do not need the TCG stub anymore. >> >> You need compile-time checks, not runtime checks. Any calls have to be >> protected by #ifdef, otherwise the compiler might bail out. > > This is not true though, tcg_enabled() is #defined as 0 if tcg is not enabled. > > #define kvm_enabled() (0) > > Compiler will elide the code if after the preprocessor pass the code is: > > if (0) { > } > Just that we are talking about the same thing: The following will fail to compile void main(void) { if (0) { return hello("Test"); } } You at least need the prototypes. But I guess we still keep them and really only remove the stubs -- which works because the linker will never stumble over them.
On 20.04.21 12:36, Claudio Fontana wrote: > now that we protect all calls to the tcg-specific functions > with if (tcg_enabled()), we do not need the TCG stub anymore. > As we keep the prototypes, this should indeed work Reviewed-by: David Hildenbrand <david@redhat.com>
On 4/20/21 3:04 PM, David Hildenbrand wrote: > On 20.04.21 15:00, Claudio Fontana wrote: >> On 4/20/21 2:54 PM, David Hildenbrand wrote: >>> On 20.04.21 12:36, Claudio Fontana wrote: >>>> now that we protect all calls to the tcg-specific functions >>>> with if (tcg_enabled()), we do not need the TCG stub anymore. >>> >>> You need compile-time checks, not runtime checks. Any calls have to be >>> protected by #ifdef, otherwise the compiler might bail out. >> >> This is not true though, tcg_enabled() is #defined as 0 if tcg is not enabled. >> >> #define kvm_enabled() (0) >> >> Compiler will elide the code if after the preprocessor pass the code is: >> >> if (0) { >> } >> > > Just that we are talking about the same thing: > > The following will fail to compile > > void main(void) > { > if (0) { > return hello("Test"); > } > } > > You at least need the prototypes. But I guess we still keep them and > really only remove the stubs -- which works because the linker will > never stumble over them. Yes, thjs is what I am saying. We have all the prototypes in sight, so we are good, no need to keep the stubs. Ciao, Claudio
diff --git a/target/s390x/tcg-stub.c b/target/s390x/tcg-stub.c deleted file mode 100644 index d22c898802..0000000000 --- a/target/s390x/tcg-stub.c +++ /dev/null @@ -1,30 +0,0 @@ -/* - * QEMU TCG support -- s390x specific function stubs. - * - * Copyright (C) 2018 Red Hat Inc - * - * Authors: - * David Hildenbrand <david@redhat.com> - * - * This work is licensed under the terms of the GNU GPL, version 2 or later. - * See the COPYING file in the top-level directory. - */ - -#include "qemu/osdep.h" -#include "qemu-common.h" -#include "cpu.h" -#include "tcg_s390x.h" - -void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque) -{ -} -void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env, - uint32_t code, uintptr_t ra) -{ - g_assert_not_reached(); -} -void QEMU_NORETURN tcg_s390_data_exception(CPUS390XState *env, uint32_t dxc, - uintptr_t ra) -{ - g_assert_not_reached(); -} diff --git a/target/s390x/meson.build b/target/s390x/meson.build index 1219f64112..a5e1ded93f 100644 --- a/target/s390x/meson.build +++ b/target/s390x/meson.build @@ -21,7 +21,7 @@ s390x_ss.add(when: 'CONFIG_TCG', if_true: files( 'vec_helper.c', 'vec_int_helper.c', 'vec_string_helper.c', -), if_false: files('tcg-stub.c')) +)) s390x_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c'))
now that we protect all calls to the tcg-specific functions with if (tcg_enabled()), we do not need the TCG stub anymore. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- target/s390x/tcg-stub.c | 30 ------------------------------ target/s390x/meson.build | 2 +- 2 files changed, 1 insertion(+), 31 deletions(-) delete mode 100644 target/s390x/tcg-stub.c