[PATCH 0/3] Cleanup includes for no-TCG builds

[PATCH 0/3] Cleanup includes for no-TCG builds

[Paolo Bonzini]

Right now, accel/tcg and tcg/$tcg_target are part of the include path for
all builds, mostly due to exec/translate-all.h and the flush_icache_range
function.  But there's also a few useless tcg/tcg.h includes that can
simply be removed.

This small series, mostly consisting of code movement and file renames,
starts cleaning up this area.

Paolo

Paolo Bonzini (3):
  remove TCG includes from common code
  move icache flush out of tcg.h
  trace: do not include TCG helper tracepoints in no-TCG builds

 accel/stubs/tcg-stub.c                      |  1 -
 accel/tcg/cputlb.c                          |  2 +-
 accel/tcg/translate-all.c                   |  2 +-
 accel/tcg/user-exec.c                       |  2 +-
 cpu.c                                       |  2 +-
 hw/i386/kvmvapic.c                          |  1 -
 include/exec/flush-icache.h                 | 64 +++++++++++++++++++++
 {accel/tcg => include/exec}/translate-all.h |  0
 monitor/misc.c                              |  1 -
 softmmu/physmem.c                           |  4 +-
 tcg/aarch64/tcg-target.h                    |  5 --
 tcg/arm/tcg-target.h                        |  5 --
 tcg/i386/tcg-target.h                       |  4 --
 tcg/mips/tcg-target.h                       | 11 ----
 tcg/ppc/tcg-target.c.inc                    | 22 --------
 tcg/ppc/tcg-target.h                        |  1 -
 tcg/riscv/tcg-target.h                      |  5 --
 tcg/s390/tcg-target.h                       |  4 --
 tcg/sparc/tcg-target.h                      |  8 ---
 tcg/tcg.c                                   |  1 +
 tcg/tci/tcg-target.h                        |  4 --
 trace/meson.build                           |  2 +-
 util/cacheinfo.c                            | 25 +++++++++
 23 files changed, 97 insertions(+), 79 deletions(-)
 create mode 100644 include/exec/flush-icache.h
 rename {accel/tcg => include/exec}/translate-all.h (100%)

-- 
2.28.0

[PATCH 1/3] remove TCG includes from common code

[Paolo Bonzini]

Enable removing tcg/$tcg_arch from the include path when TCG is disabled.
Move translate-all.h to include/exec, since stubs exist for the functions
defined therein.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/stubs/tcg-stub.c                      | 1 -
 accel/tcg/cputlb.c                          | 2 +-
 accel/tcg/translate-all.c                   | 2 +-
 accel/tcg/user-exec.c                       | 2 +-
 cpu.c                                       | 2 +-
 hw/i386/kvmvapic.c                          | 1 -
 {accel/tcg => include/exec}/translate-all.h | 0
 monitor/misc.c                              | 1 -
 softmmu/physmem.c                           | 3 +--
 9 files changed, 5 insertions(+), 9 deletions(-)
 rename {accel/tcg => include/exec}/translate-all.h (100%)

diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
index e4bbf997aa..8c18d3eabd 100644
--- a/accel/stubs/tcg-stub.c
+++ b/accel/stubs/tcg-stub.c
@@ -12,7 +12,6 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "tcg/tcg.h"
 #include "exec/exec-all.h"
 
 void tb_flush(CPUState *cpu)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 42ab79c1a5..ced3dc077e 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -33,7 +33,7 @@
 #include "exec/helper-proto.h"
 #include "qemu/atomic.h"
 #include "qemu/atomic128.h"
-#include "translate-all.h"
+#include "exec/translate-all.h"
 #include "trace/trace-root.h"
 #include "trace/mem.h"
 #ifdef CONFIG_PLUGIN
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index b7d50a73d4..a1803a1026 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -49,7 +49,7 @@
 
 #include "exec/cputlb.h"
 #include "exec/tb-hash.h"
-#include "translate-all.h"
+#include "exec/translate-all.h"
 #include "qemu/bitmap.h"
 #include "qemu/error-report.h"
 #include "qemu/qemu-print.h"
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 4ebe25461a..abb9555201 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -23,7 +23,7 @@
 #include "tcg/tcg.h"
 #include "qemu/bitops.h"
 #include "exec/cpu_ldst.h"
-#include "translate-all.h"
+#include "exec/translate-all.h"
 #include "exec/helper-proto.h"
 #include "qemu/atomic128.h"
 #include "trace/trace-root.h"
diff --git a/cpu.c b/cpu.c
index 0c485cdf2d..0b245cda2e 100644
--- a/cpu.c
+++ b/cpu.c
@@ -34,7 +34,7 @@
 #include "sysemu/tcg.h"
 #include "sysemu/kvm.h"
 #include "sysemu/replay.h"
-#include "translate-all.h"
+#include "exec/translate-all.h"
 #include "exec/log.h"
 
 uintptr_t qemu_host_page_size;
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 077c3f4866..2c1898032e 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -21,7 +21,6 @@
 #include "hw/sysbus.h"
 #include "hw/boards.h"
 #include "migration/vmstate.h"
-#include "tcg/tcg.h"
 #include "qom/object.h"
 
 #define VAPIC_IO_PORT           0x7e
diff --git a/accel/tcg/translate-all.h b/include/exec/translate-all.h
similarity index 100%
rename from accel/tcg/translate-all.h
rename to include/exec/translate-all.h
diff --git a/monitor/misc.c b/monitor/misc.c
index fde6e36a0b..9a132f73d1 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -77,7 +77,6 @@
 #include "qapi/qmp-event.h"
 #include "sysemu/cpus.h"
 #include "qemu/cutils.h"
-#include "tcg/tcg.h"
 
 #if defined(TARGET_S390X)
 #include "hw/s390x/storage-keys.h"
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 2cd1de4a2c..67b53d39e4 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -25,7 +25,6 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/target_page.h"
-#include "tcg/tcg.h"
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
 #include "hw/boards.h"
@@ -53,7 +52,7 @@
 
 #include "qemu/rcu_queue.h"
 #include "qemu/main-loop.h"
-#include "translate-all.h"
+#include "exec/translate-all.h"
 #include "sysemu/replay.h"
 
 #include "exec/memory-internal.h"
-- 
2.28.0

[PATCH 2/3] move icache flush out of tcg.h

[Paolo Bonzini]

icache flush is also used by non-TCG accelerators.  Do not require users
such as physmem.c to include tcg/tcg.h.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/flush-icache.h | 64 ++++++++++++++++++++++++++++++++++++++++
 softmmu/physmem.c           |  1 +
 tcg/aarch64/tcg-target.h    |  5 ----
 tcg/arm/tcg-target.h        |  5 ----
 tcg/i386/tcg-target.h       |  4 ---
 tcg/mips/tcg-target.h       | 11 -------
 tcg/ppc/tcg-target.c.inc    | 22 --------------
 tcg/ppc/tcg-target.h        |  1 -
 tcg/riscv/tcg-target.h      |  5 ----
 tcg/s390/tcg-target.h       |  4 ---
 tcg/sparc/tcg-target.h      |  8 -----
 tcg/tcg.c                   |  1 +
 tcg/tci/tcg-target.h        |  4 ---
 util/cacheinfo.c            | 25 ++++++++++++++++
 14 files changed, 91 insertions(+), 69 deletions(-)
 create mode 100644 include/exec/flush-icache.h

diff --git a/include/exec/flush-icache.h b/include/exec/flush-icache.h
new file mode 100644
index 0000000000..77a7b80ab7
--- /dev/null
+++ b/include/exec/flush-icache.h
@@ -0,0 +1,64 @@
+/*
+ * icache flush for QEMU (both TCG or virtualized)
+ *
+ *  Copyright (c) 2003-2020 QEMU contributors
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef FLUSH_ICACHE_H
+#define FLUSH_ICACHE_H
+
+#if defined HOST_I386 || defined HOST_X86_64 || defined HOST_S390 || defined HOST_S390X
+static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
+{
+}
+
+#elif defined HOST_MIPS
+#ifdef __OpenBSD__
+#include <machine/sysarch.h>
+#else
+#include <sys/cachectl.h>
+#endif
+
+static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
+{
+    cacheflush ((void *)start, stop-start, ICACHE);
+}
+
+#elif defined HOST_PPC || defined HOST_PPC64
+extern void flush_icache_range(uintptr_t start, uintptr_t stop);
+
+#elif defined HOST_SPARC
+static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
+{
+    uintptr_t p;
+    for (p = start & -8; p < ((stop + 7) & -8); p += 8) {
+        __asm__ __volatile__("flush\t%0" : : "r" (p));
+    }
+}
+
+#elif defined HOST_AARCH64 || defined HOST_ARM || defined HOST_RISCV
+static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
+{
+    __builtin___clear_cache((char *)start, (char *)stop);
+}
+#elif defined CONFIG_TCG_INTERPRETER
+static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
+{
+    assert(tcg_enabled());
+}
+#endif
+
+#endif
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 67b53d39e4..c6a3cb679e 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -53,6 +53,7 @@
 #include "qemu/rcu_queue.h"
 #include "qemu/main-loop.h"
 #include "exec/translate-all.h"
+#include "exec/flush-icache.h"
 #include "sysemu/replay.h"
 
 #include "exec/memory-internal.h"
diff --git a/tcg/aarch64/tcg-target.h b/tcg/aarch64/tcg-target.h
index 663dd0b95e..8a6b97598e 100644
--- a/tcg/aarch64/tcg-target.h
+++ b/tcg/aarch64/tcg-target.h
@@ -148,11 +148,6 @@ typedef enum {
 #define TCG_TARGET_DEFAULT_MO (0)
 #define TCG_TARGET_HAS_MEMORY_BSWAP     1
 
-static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
-{
-    __builtin___clear_cache((char *)start, (char *)stop);
-}
-
 void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t);
 
 #ifdef CONFIG_SOFTMMU
diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
index 17e771374d..f1955ce4ac 100644
--- a/tcg/arm/tcg-target.h
+++ b/tcg/arm/tcg-target.h
@@ -134,11 +134,6 @@ enum {
 #define TCG_TARGET_DEFAULT_MO (0)
 #define TCG_TARGET_HAS_MEMORY_BSWAP     1
 
-static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
-{
-    __builtin___clear_cache((char *) start, (char *) stop);
-}
-
 /* not defined -- call should be eliminated at compile time */
 void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t);
 
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index abd4ac7fc0..cd067e0b30 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -206,10 +206,6 @@ extern bool have_avx2;
 #define TCG_TARGET_extract_i64_valid(ofs, len) \
     (((ofs) == 8 && (len) == 8) || ((ofs) + (len)) == 32)
 
-static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
-{
-}
-
 static inline void tb_target_set_jmp_target(uintptr_t tc_ptr,
                                             uintptr_t jmp_addr, uintptr_t addr)
 {
diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h
index c6b091d849..92c1d63da3 100644
--- a/tcg/mips/tcg-target.h
+++ b/tcg/mips/tcg-target.h
@@ -198,20 +198,9 @@ extern bool use_mips32r2_instructions;
 #define TCG_TARGET_HAS_ext16u_i64       0 /* andi rt, rs, 0xffff */
 #endif
 
-#ifdef __OpenBSD__
-#include <machine/sysarch.h>
-#else
-#include <sys/cachectl.h>
-#endif
-
 #define TCG_TARGET_DEFAULT_MO (0)
 #define TCG_TARGET_HAS_MEMORY_BSWAP     1
 
-static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
-{
-    cacheflush ((void *)start, stop-start, ICACHE);
-}
-
 void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t);
 
 #ifdef CONFIG_SOFTMMU
diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
index 18ee989f95..0d068ec8ab 100644
--- a/tcg/ppc/tcg-target.c.inc
+++ b/tcg/ppc/tcg-target.c.inc
@@ -3863,25 +3863,3 @@ void tcg_register_jit(void *buf, size_t buf_size)
     tcg_register_jit_int(buf, buf_size, &debug_frame, sizeof(debug_frame));
 }
 #endif /* __ELF__ */
-
-void flush_icache_range(uintptr_t start, uintptr_t stop)
-{
-    uintptr_t p, start1, stop1;
-    size_t dsize = qemu_dcache_linesize;
-    size_t isize = qemu_icache_linesize;
-
-    start1 = start & ~(dsize - 1);
-    stop1 = (stop + dsize - 1) & ~(dsize - 1);
-    for (p = start1; p < stop1; p += dsize) {
-        asm volatile ("dcbst 0,%0" : : "r"(p) : "memory");
-    }
-    asm volatile ("sync" : : : "memory");
-
-    start &= start & ~(isize - 1);
-    stop1 = (stop + isize - 1) & ~(isize - 1);
-    for (p = start1; p < stop1; p += isize) {
-        asm volatile ("icbi 0,%0" : : "r"(p) : "memory");
-    }
-    asm volatile ("sync" : : : "memory");
-    asm volatile ("isync" : : : "memory");
-}
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index be10363956..a509a19628 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -175,7 +175,6 @@ extern bool have_vsx;
 #define TCG_TARGET_HAS_bitsel_vec       have_vsx
 #define TCG_TARGET_HAS_cmpsel_vec       0
 
-void flush_icache_range(uintptr_t start, uintptr_t stop);
 void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t);
 
 #define TCG_TARGET_DEFAULT_MO (0)
diff --git a/tcg/riscv/tcg-target.h b/tcg/riscv/tcg-target.h
index 032439d806..c1bd52bb9a 100644
--- a/tcg/riscv/tcg-target.h
+++ b/tcg/riscv/tcg-target.h
@@ -159,11 +159,6 @@ typedef enum {
 #define TCG_TARGET_HAS_mulsh_i64        1
 #endif
 
-static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
-{
-    __builtin___clear_cache((char *)start, (char *)stop);
-}
-
 /* not defined -- call should be eliminated at compile time */
 void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t);
 
diff --git a/tcg/s390/tcg-target.h b/tcg/s390/tcg-target.h
index 63c8797bd3..b4feb2f55a 100644
--- a/tcg/s390/tcg-target.h
+++ b/tcg/s390/tcg-target.h
@@ -145,10 +145,6 @@ enum {
     TCG_AREG0 = TCG_REG_R10,
 };
 
-static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
-{
-}
-
 static inline void tb_target_set_jmp_target(uintptr_t tc_ptr,
                                             uintptr_t jmp_addr, uintptr_t addr)
 {
diff --git a/tcg/sparc/tcg-target.h b/tcg/sparc/tcg-target.h
index 633841ebf2..d8b0e32e2e 100644
--- a/tcg/sparc/tcg-target.h
+++ b/tcg/sparc/tcg-target.h
@@ -168,14 +168,6 @@ extern bool use_vis3_instructions;
 #define TCG_TARGET_DEFAULT_MO (0)
 #define TCG_TARGET_HAS_MEMORY_BSWAP     1
 
-static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
-{
-    uintptr_t p;
-    for (p = start & -8; p < ((stop + 7) & -8); p += 8) {
-        __asm__ __volatile__("flush\t%0" : : "r" (p));
-    }
-}
-
 void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t);
 
 #define TCG_TARGET_NEED_POOL_LABELS
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 43c6cf8f52..9974bb3c56 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -43,6 +43,7 @@
 #include "cpu.h"
 
 #include "exec/exec-all.h"
+#include "exec/flush-icache.h"
 
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/boards.h"
diff --git a/tcg/tci/tcg-target.h b/tcg/tci/tcg-target.h
index 8c1c1d265d..b84480f989 100644
--- a/tcg/tci/tcg-target.h
+++ b/tcg/tci/tcg-target.h
@@ -191,10 +191,6 @@ void tci_disas(uint8_t opc);
 
 #define HAVE_TCG_QEMU_TB_EXEC
 
-static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
-{
-}
-
 /* We could notice __i386__ or __s390x__ and reduce the barriers depending
    on the host.  But if you want performance, you use the normal backend.
    We prefer consistency across hosts on this.  */
diff --git a/util/cacheinfo.c b/util/cacheinfo.c
index 7804c186b6..b49093106d 100644
--- a/util/cacheinfo.c
+++ b/util/cacheinfo.c
@@ -9,6 +9,7 @@
 #include "qemu/osdep.h"
 #include "qemu/host-utils.h"
 #include "qemu/atomic.h"
+#include "exec/flush-icache.h"
 
 int qemu_icache_linesize = 0;
 int qemu_icache_linesize_log;
@@ -177,6 +178,30 @@ static void fallback_cache_info(int *isize, int *dsize)
     }
 }
 
+#if defined HOST_PPC || defined HOST_PPC64
+void flush_icache_range(uintptr_t start, uintptr_t stop)
+{
+    uintptr_t p, start1, stop1;
+    size_t dsize = qemu_dcache_linesize;
+    size_t isize = qemu_icache_linesize;
+
+    start1 = start & ~(dsize - 1);
+    stop1 = (stop + dsize - 1) & ~(dsize - 1);
+    for (p = start1; p < stop1; p += dsize) {
+        asm volatile ("dcbst 0,%0" : : "r"(p) : "memory");
+    }
+    asm volatile ("sync" : : : "memory");
+
+    start &= start & ~(isize - 1);
+    stop1 = (stop + isize - 1) & ~(isize - 1);
+    for (p = start1; p < stop1; p += isize) {
+        asm volatile ("icbi 0,%0" : : "r"(p) : "memory");
+    }
+    asm volatile ("sync" : : : "memory");
+    asm volatile ("isync" : : : "memory");
+}
+#endif
+
 static void __attribute__((constructor)) init_cache_info(void)
 {
     int isize = 0, dsize = 0;
-- 
2.28.0

[PATCH 3/3] trace: do not include TCG helper tracepoints in no-TCG builds

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 trace/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/trace/meson.build b/trace/meson.build
index 3fa760f568..012d0dbceb 100644
--- a/trace/meson.build
+++ b/trace/meson.build
@@ -71,7 +71,7 @@ foreach d : [
                 input: meson.project_source_root() / 'trace-events',
                 command: [ tracetool, '--group=root', '--format=@0@'.format(d[1]), '@INPUT@' ],
                 capture: true)
-  specific_ss.add(gen)
+  specific_ss.add(when: 'CONFIG_TCG', if_true: gen)
 endforeach
 
 if 'CONFIG_TRACE_UST' in config_host
-- 
2.28.0

Re: [PATCH 2/3] move icache flush out of tcg.h

[Richard Henderson]

On 12/16/20 7:55 AM, Paolo Bonzini wrote:
> icache flush is also used by non-TCG accelerators.  Do not require users
> such as physmem.c to include tcg/tcg.h.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

See also

https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg03623.html

I moved it to a new file, rather than into cacheinfo.c.


r~

Re: [PATCH 2/3] move icache flush out of tcg.h

[Paolo Bonzini]

On 16/12/20 15:52, Richard Henderson wrote:
> On 12/16/20 7:55 AM, Paolo Bonzini wrote:
>> icache flush is also used by non-TCG accelerators.  Do not require users
>> such as physmem.c to include tcg/tcg.h.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
> 
> See also
> 
> https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg03623.html
> 
> I moved it to a new file, rather than into cacheinfo.c.

Oh, I missed that.  I'll pick your patch instead of this one.

Paolo

Re: [PATCH 1/3] remove TCG includes from common code

[Richard Henderson]

On 12/16/20 7:55 AM, Paolo Bonzini wrote:
> Enable removing tcg/$tcg_arch from the include path when TCG is disabled.
> Move translate-all.h to include/exec, since stubs exist for the functions
> defined therein.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  accel/stubs/tcg-stub.c                      | 1 -
>  accel/tcg/cputlb.c                          | 2 +-
>  accel/tcg/translate-all.c                   | 2 +-
>  accel/tcg/user-exec.c                       | 2 +-
>  cpu.c                                       | 2 +-
>  hw/i386/kvmvapic.c                          | 1 -
>  {accel/tcg => include/exec}/translate-all.h | 0
>  monitor/misc.c                              | 1 -
>  softmmu/physmem.c                           | 3 +--
>  9 files changed, 5 insertions(+), 9 deletions(-)
>  rename {accel/tcg => include/exec}/translate-all.h (100%)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/3] trace: do not include TCG helper tracepoints in no-TCG builds

[Richard Henderson]

On 12/16/20 7:55 AM, Paolo Bonzini wrote:
> diff --git a/trace/meson.build b/trace/meson.build
> index 3fa760f568..012d0dbceb 100644
> --- a/trace/meson.build
> +++ b/trace/meson.build
> @@ -71,7 +71,7 @@ foreach d : [
>                  input: meson.project_source_root() / 'trace-events',
>                  command: [ tracetool, '--group=root', '--format=@0@'.format(d[1]), '@INPUT@' ],
>                  capture: true)
> -  specific_ss.add(gen)
> +  specific_ss.add(when: 'CONFIG_TCG', if_true: gen)
>  endforeach

Sure, unless it makes more sense to conditionalize the entire loop, so that
those targets aren't even created?

Acked-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 3/3] trace: do not include TCG helper tracepoints in no-TCG builds

[Paolo Bonzini]

On 16/12/20 16:13, Richard Henderson wrote:
>> @@ -71,7 +71,7 @@ foreach d : [
>>                   input: meson.project_source_root() / 'trace-events',
>>                   command: [ tracetool, '--group=root', '--format=@0@'.format(d[1]), '@INPUT@' ],
>>                   capture: true)
>> -  specific_ss.add(gen)
>> +  specific_ss.add(when: 'CONFIG_TCG', if_true: gen)
>>   endforeach
>
> Sure, unless it makes more sense to conditionalize the entire loop, so that
> those targets aren't even created?

The targets exist but the files won't be created (the rules will be in 
build.ninja but they'll be orphan).

Paolo

Re: [PATCH 3/3] trace: do not include TCG helper tracepoints in no-TCG builds

[Richard Henderson]

On 12/16/20 9:22 AM, Paolo Bonzini wrote:
> On 16/12/20 16:13, Richard Henderson wrote:
>>> @@ -71,7 +71,7 @@ foreach d : [
>>>                   input: meson.project_source_root() / 'trace-events',
>>>                   command: [ tracetool, '--group=root',
>>> '--format=@0@'.format(d[1]), '@INPUT@' ],
>>>                   capture: true)
>>> -  specific_ss.add(gen)
>>> +  specific_ss.add(when: 'CONFIG_TCG', if_true: gen)
>>>   endforeach
>>
>> Sure, unless it makes more sense to conditionalize the entire loop, so that
>> those targets aren't even created?
> 
> The targets exist but the files won't be created (the rules will be in
> build.ninja but they'll be orphan).

Yes, I get that.  So... is there a reason for them to be there, when it seems
just as easy for them not to be?


r~

Re: [PATCH 1/3] remove TCG includes from common code

[Philippe Mathieu-Daudé]

On 12/16/20 2:55 PM, Paolo Bonzini wrote:
> Enable removing tcg/$tcg_arch from the include path when TCG is disabled.
> Move translate-all.h to include/exec, since stubs exist for the functions
> defined therein.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  accel/stubs/tcg-stub.c                      | 1 -
>  accel/tcg/cputlb.c                          | 2 +-
>  accel/tcg/translate-all.c                   | 2 +-
>  accel/tcg/user-exec.c                       | 2 +-
>  cpu.c                                       | 2 +-
>  hw/i386/kvmvapic.c                          | 1 -
>  {accel/tcg => include/exec}/translate-all.h | 0
>  monitor/misc.c                              | 1 -
>  softmmu/physmem.c                           | 3 +--
>  9 files changed, 5 insertions(+), 9 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 3/3] trace: do not include TCG helper tracepoints in no-TCG builds

[Paolo Bonzini]

On 16/12/20 16:25, Richard Henderson wrote:
> On 12/16/20 9:22 AM, Paolo Bonzini wrote:
>> On 16/12/20 16:13, Richard Henderson wrote:
>>>> @@ -71,7 +71,7 @@ foreach d : [
>>>>                    input: meson.project_source_root() / 'trace-events',
>>>>                    command: [ tracetool, '--group=root',
>>>> '--format=@0@'.format(d[1]), '@INPUT@' ],
>>>>                    capture: true)
>>>> -  specific_ss.add(gen)
>>>> +  specific_ss.add(when: 'CONFIG_TCG', if_true: gen)
>>>>    endforeach
>>>
>>> Sure, unless it makes more sense to conditionalize the entire loop, so that
>>> those targets aren't even created?
>>
>> The targets exist but the files won't be created (the rules will be in
>> build.ninja but they'll be orphan).
> 
> Yes, I get that.  So... is there a reason for them to be there, when it seems
> just as easy for them not to be?

Just a dislike for "if"s when conditional add() will do.

Speaking in general, I like the various sections of the meson.build 
files to use either foreach (taking the job of Makefile target pattern 
rules, where "if"s add complication) or if (taking the job of configure 
code, where loops are rare).  It can't always be done, but it helps 
limiting the amount of spaghetti...

Paolo