Re: [PATCH-for-9.1 22/27] target/s390x: Convert to TCGCPUOps::get_cpu_state()
On 3/19/24 21:09, Philippe Mathieu-Daudé wrote: On 19/3/24 22:05, Richard Henderson wrote: On 3/19/24 05:42, Philippe Mathieu-Daudé wrote: Convert cpu_get_tb_cpu_state() to TCGCPUOps::get_cpu_state(). Note, now s390x_get_cpu_state() is restricted to TCG. Signed-off-by: Philippe Mathieu-Daudé --- target/s390x/cpu.h | 30 -- target/s390x/s390x-internal.h | 2 ++ target/s390x/cpu.c | 1 + target/s390x/tcg/mem_helper.c | 2 +- target/s390x/tcg/translate.c | 23 +++ 5 files changed, 27 insertions(+), 31 deletions(-) Why is the function in translate.c, not cpu.c (with or without ifdefs)? My understanding is target/foo/tcg/ is better for TCG-specific handlers, less #ifdef'ry and stubs. Then bar_helper.c are meant for TCG helpers (including "exec/helper-proto.h"). Can you think of a better file (new name?) in tcg/ or do you rather keep it in the main cpu.c? Given that all other targets to this point used cpu.c, I would prefer s390x and sparc to not be the only exceptions. r~
Re: [PATCH-for-9.1 22/27] target/s390x: Convert to TCGCPUOps::get_cpu_state()
On 19/3/24 22:05, Richard Henderson wrote: On 3/19/24 05:42, Philippe Mathieu-Daudé wrote: Convert cpu_get_tb_cpu_state() to TCGCPUOps::get_cpu_state(). Note, now s390x_get_cpu_state() is restricted to TCG. Signed-off-by: Philippe Mathieu-Daudé --- target/s390x/cpu.h | 30 -- target/s390x/s390x-internal.h | 2 ++ target/s390x/cpu.c | 1 + target/s390x/tcg/mem_helper.c | 2 +- target/s390x/tcg/translate.c | 23 +++ 5 files changed, 27 insertions(+), 31 deletions(-) Why is the function in translate.c, not cpu.c (with or without ifdefs)? My understanding is target/foo/tcg/ is better for TCG-specific handlers, less #ifdef'ry and stubs. Then bar_helper.c are meant for TCG helpers (including "exec/helper-proto.h"). Can you think of a better file (new name?) in tcg/ or do you rather keep it in the main cpu.c? Thanks, Phil.
Re: [PATCH-for-9.1 22/27] target/s390x: Convert to TCGCPUOps::get_cpu_state()
On 3/19/24 05:42, Philippe Mathieu-Daudé wrote: Convert cpu_get_tb_cpu_state() to TCGCPUOps::get_cpu_state(). Note, now s390x_get_cpu_state() is restricted to TCG. Signed-off-by: Philippe Mathieu-Daudé --- target/s390x/cpu.h| 30 -- target/s390x/s390x-internal.h | 2 ++ target/s390x/cpu.c| 1 + target/s390x/tcg/mem_helper.c | 2 +- target/s390x/tcg/translate.c | 23 +++ 5 files changed, 27 insertions(+), 31 deletions(-) Why is the function in translate.c, not cpu.c (with or without ifdefs)? r~
