diff mbox series

[RFC,v2,04/13] target/s390x: remove tcg-stub.c

Message ID 20210420103616.32731-5-cfontana@suse.de (mailing list archive)
State New
Headers show
Series s390x cleanup | expand

Commit Message

Claudio Fontana April 20, 2021, 10:36 a.m. UTC
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

Comments

David Hildenbrand April 20, 2021, 12:54 p.m. UTC | #1
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.
Claudio Fontana April 20, 2021, 1 p.m. UTC | #2
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
David Hildenbrand April 20, 2021, 1:04 p.m. UTC | #3
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.
David Hildenbrand April 20, 2021, 1:06 p.m. UTC | #4
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>
Claudio Fontana April 20, 2021, 1:35 p.m. UTC | #5
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 mbox series

Patch

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'))