[PATCH v2] qemu/compiler: Absorb 'clang-tsa.h'

[PATCH v2] qemu/compiler: Absorb 'clang-tsa.h'

[Philippe Mathieu-Daudé]

We already have "qemu/compiler.h" for compiler-specific arrangements,
automatically included by "qemu/osdep.h" for each source file. No
need to explicitly include a header for a Clang particularity,
let the common "qemu/compiler.h" deal with that by having it
include "qemu/clang-tsa.h" (renamed as qemu/clang-tsa.h.inc).
Add a check to not include "qemu/clang-tsa.h.inc" directly,
remove previous "qemu/clang-tsa.h" inclusions.

Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 bsd-user/qemu.h                               | 1 -
 include/block/block_int-common.h              | 1 -
 include/block/graph-lock.h                    | 2 --
 include/exec/page-protection.h                | 2 --
 include/qemu/compiler.h                       | 2 ++
 include/qemu/thread.h                         | 1 -
 include/qemu/{clang-tsa.h => clang-tsa.h.inc} | 8 ++++++--
 block/create.c                                | 1 -
 tests/unit/test-bdrv-drain.c                  | 1 -
 tests/unit/test-block-iothread.c              | 1 -
 util/qemu-thread-posix.c                      | 1 -
 11 files changed, 8 insertions(+), 13 deletions(-)
 rename include/qemu/{clang-tsa.h => clang-tsa.h.inc} (97%)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 3eaa14f3f56..4e97c796318 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -40,7 +40,6 @@ extern char **environ;
 #include "target.h"
 #include "exec/gdbstub.h"
 #include "exec/page-protection.h"
-#include "qemu/clang-tsa.h"
 #include "accel/tcg/vcpu-state.h"
 
 #include "qemu-os.h"
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index bb91a0f62fa..ebb4e56a503 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -28,7 +28,6 @@
 #include "block/block-common.h"
 #include "block/block-global-state.h"
 #include "block/snapshot.h"
-#include "qemu/clang-tsa.h"
 #include "qemu/iov.h"
 #include "qemu/rcu.h"
 #include "qemu/stats64.h"
diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h
index dc8d9491843..2c26c721081 100644
--- a/include/block/graph-lock.h
+++ b/include/block/graph-lock.h
@@ -20,8 +20,6 @@
 #ifndef GRAPH_LOCK_H
 #define GRAPH_LOCK_H
 
-#include "qemu/clang-tsa.h"
-
 /**
  * Graph Lock API
  * This API provides a rwlock used to protect block layer
diff --git a/include/exec/page-protection.h b/include/exec/page-protection.h
index bae3355f62c..3e0a8a03331 100644
--- a/include/exec/page-protection.h
+++ b/include/exec/page-protection.h
@@ -40,8 +40,6 @@
 
 #ifdef CONFIG_USER_ONLY
 
-#include "qemu/clang-tsa.h"
-
 void TSA_NO_TSA mmap_lock(void);
 void TSA_NO_TSA mmap_unlock(void);
 bool have_mmap_lock(void);
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index c06954ccb41..6dfd8da14d9 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -7,6 +7,8 @@
 #ifndef COMPILER_H
 #define COMPILER_H
 
+#include "qemu/clang-tsa.h.inc"
+
 #define HOST_BIG_ENDIAN (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
 
 /* HOST_LONG_BITS is the size of a native pointer in bits. */
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 7eba27a7049..6f800aad31a 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -3,7 +3,6 @@
 
 #include "qemu/processor.h"
 #include "qemu/atomic.h"
-#include "qemu/clang-tsa.h"
 
 typedef struct QemuCond QemuCond;
 typedef struct QemuSemaphore QemuSemaphore;
diff --git a/include/qemu/clang-tsa.h b/include/qemu/clang-tsa.h.inc
similarity index 97%
rename from include/qemu/clang-tsa.h
rename to include/qemu/clang-tsa.h.inc
index ba06fb8c924..1273c39f9c5 100644
--- a/include/qemu/clang-tsa.h
+++ b/include/qemu/clang-tsa.h.inc
@@ -1,5 +1,5 @@
-#ifndef CLANG_TSA_H
-#define CLANG_TSA_H
+#ifndef CLANG_TSA_H_INC
+#define CLANG_TSA_H_INC
 
 /*
  * Copyright 2018 Jarkko Hietaniemi <jhi@iki.fi>
@@ -24,6 +24,10 @@
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
+#ifndef COMPILER_H
+#error Cannot include this header directly
+#endif
+
 /* http://clang.llvm.org/docs/ThreadSafetyAnalysis.html
  *
  * TSA is available since clang 3.6-ish.
diff --git a/block/create.c b/block/create.c
index 72abafb4c12..6b23a216753 100644
--- a/block/create.c
+++ b/block/create.c
@@ -24,7 +24,6 @@
 
 #include "qemu/osdep.h"
 #include "block/block_int.h"
-#include "qemu/clang-tsa.h"
 #include "qemu/job.h"
 #include "qemu/main-loop.h"
 #include "qapi/qapi-commands-block-core.h"
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index 98ad89b390c..7410e6f3528 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -28,7 +28,6 @@
 #include "system/block-backend.h"
 #include "qapi/error.h"
 #include "qemu/main-loop.h"
-#include "qemu/clang-tsa.h"
 #include "iothread.h"
 
 static QemuEvent done_event;
diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index 1de04a8a13d..26a6c051758 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -29,7 +29,6 @@
 #include "system/block-backend.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qdict.h"
-#include "qemu/clang-tsa.h"
 #include "qemu/main-loop.h"
 #include "iothread.h"
 
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 6fff4162ac6..b2e26e21205 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -17,7 +17,6 @@
 #include "qemu-thread-common.h"
 #include "qemu/tsan.h"
 #include "qemu/bitmap.h"
-#include "qemu/clang-tsa.h"
 
 #ifdef CONFIG_PTHREAD_SET_NAME_NP
 #include <pthread_np.h>
-- 
2.47.1

Re: [PATCH v2] qemu/compiler: Absorb 'clang-tsa.h'

[Pierrick Bouvier]

On 1/16/25 13:11, Philippe Mathieu-Daudé wrote:
> We already have "qemu/compiler.h" for compiler-specific arrangements,
> automatically included by "qemu/osdep.h" for each source file. No
> need to explicitly include a header for a Clang particularity,
> let the common "qemu/compiler.h" deal with that by having it
> include "qemu/clang-tsa.h" (renamed as qemu/clang-tsa.h.inc).
> Add a check to not include "qemu/clang-tsa.h.inc" directly,
> remove previous "qemu/clang-tsa.h" inclusions.
> 
> Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   bsd-user/qemu.h                               | 1 -
>   include/block/block_int-common.h              | 1 -
>   include/block/graph-lock.h                    | 2 --
>   include/exec/page-protection.h                | 2 --
>   include/qemu/compiler.h                       | 2 ++
>   include/qemu/thread.h                         | 1 -
>   include/qemu/{clang-tsa.h => clang-tsa.h.inc} | 8 ++++++--
>   block/create.c                                | 1 -
>   tests/unit/test-bdrv-drain.c                  | 1 -
>   tests/unit/test-block-iothread.c              | 1 -
>   util/qemu-thread-posix.c                      | 1 -
>   11 files changed, 8 insertions(+), 13 deletions(-)
>   rename include/qemu/{clang-tsa.h => clang-tsa.h.inc} (97%)
> 
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index 3eaa14f3f56..4e97c796318 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -40,7 +40,6 @@ extern char **environ;
>   #include "target.h"
>   #include "exec/gdbstub.h"
>   #include "exec/page-protection.h"
> -#include "qemu/clang-tsa.h"
>   #include "accel/tcg/vcpu-state.h"
>   
>   #include "qemu-os.h"
> diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
> index bb91a0f62fa..ebb4e56a503 100644
> --- a/include/block/block_int-common.h
> +++ b/include/block/block_int-common.h
> @@ -28,7 +28,6 @@
>   #include "block/block-common.h"
>   #include "block/block-global-state.h"
>   #include "block/snapshot.h"
> -#include "qemu/clang-tsa.h"
>   #include "qemu/iov.h"
>   #include "qemu/rcu.h"
>   #include "qemu/stats64.h"
> diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h
> index dc8d9491843..2c26c721081 100644
> --- a/include/block/graph-lock.h
> +++ b/include/block/graph-lock.h
> @@ -20,8 +20,6 @@
>   #ifndef GRAPH_LOCK_H
>   #define GRAPH_LOCK_H
>   
> -#include "qemu/clang-tsa.h"
> -
>   /**
>    * Graph Lock API
>    * This API provides a rwlock used to protect block layer
> diff --git a/include/exec/page-protection.h b/include/exec/page-protection.h
> index bae3355f62c..3e0a8a03331 100644
> --- a/include/exec/page-protection.h
> +++ b/include/exec/page-protection.h
> @@ -40,8 +40,6 @@
>   
>   #ifdef CONFIG_USER_ONLY
>   
> -#include "qemu/clang-tsa.h"
> -
>   void TSA_NO_TSA mmap_lock(void);
>   void TSA_NO_TSA mmap_unlock(void);
>   bool have_mmap_lock(void);
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index c06954ccb41..6dfd8da14d9 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -7,6 +7,8 @@
>   #ifndef COMPILER_H
>   #define COMPILER_H
>   
> +#include "qemu/clang-tsa.h.inc"
> +
>   #define HOST_BIG_ENDIAN (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
>   
>   /* HOST_LONG_BITS is the size of a native pointer in bits. */
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index 7eba27a7049..6f800aad31a 100644
> --- a/include/qemu/thread.h
> +++ b/include/qemu/thread.h
> @@ -3,7 +3,6 @@
>   
>   #include "qemu/processor.h"
>   #include "qemu/atomic.h"
> -#include "qemu/clang-tsa.h"
>   
>   typedef struct QemuCond QemuCond;
>   typedef struct QemuSemaphore QemuSemaphore;
> diff --git a/include/qemu/clang-tsa.h b/include/qemu/clang-tsa.h.inc
> similarity index 97%
> rename from include/qemu/clang-tsa.h
> rename to include/qemu/clang-tsa.h.inc
> index ba06fb8c924..1273c39f9c5 100644
> --- a/include/qemu/clang-tsa.h
> +++ b/include/qemu/clang-tsa.h.inc
> @@ -1,5 +1,5 @@
> -#ifndef CLANG_TSA_H
> -#define CLANG_TSA_H
> +#ifndef CLANG_TSA_H_INC
> +#define CLANG_TSA_H_INC
>   
>   /*
>    * Copyright 2018 Jarkko Hietaniemi <jhi@iki.fi>
> @@ -24,6 +24,10 @@
>    * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>    */
>   
> +#ifndef COMPILER_H
> +#error Cannot include this header directly
> +#endif
> +
>   /* http://clang.llvm.org/docs/ThreadSafetyAnalysis.html
>    *
>    * TSA is available since clang 3.6-ish.
> diff --git a/block/create.c b/block/create.c
> index 72abafb4c12..6b23a216753 100644
> --- a/block/create.c
> +++ b/block/create.c
> @@ -24,7 +24,6 @@
>   
>   #include "qemu/osdep.h"
>   #include "block/block_int.h"
> -#include "qemu/clang-tsa.h"
>   #include "qemu/job.h"
>   #include "qemu/main-loop.h"
>   #include "qapi/qapi-commands-block-core.h"
> diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
> index 98ad89b390c..7410e6f3528 100644
> --- a/tests/unit/test-bdrv-drain.c
> +++ b/tests/unit/test-bdrv-drain.c
> @@ -28,7 +28,6 @@
>   #include "system/block-backend.h"
>   #include "qapi/error.h"
>   #include "qemu/main-loop.h"
> -#include "qemu/clang-tsa.h"
>   #include "iothread.h"
>   
>   static QemuEvent done_event;
> diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
> index 1de04a8a13d..26a6c051758 100644
> --- a/tests/unit/test-block-iothread.c
> +++ b/tests/unit/test-block-iothread.c
> @@ -29,7 +29,6 @@
>   #include "system/block-backend.h"
>   #include "qapi/error.h"
>   #include "qapi/qmp/qdict.h"
> -#include "qemu/clang-tsa.h"
>   #include "qemu/main-loop.h"
>   #include "iothread.h"
>   
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 6fff4162ac6..b2e26e21205 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -17,7 +17,6 @@
>   #include "qemu-thread-common.h"
>   #include "qemu/tsan.h"
>   #include "qemu/bitmap.h"
> -#include "qemu/clang-tsa.h"
>   
>   #ifdef CONFIG_PTHREAD_SET_NAME_NP
>   #include <pthread_np.h>

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH v2] qemu/compiler: Absorb 'clang-tsa.h'

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> We already have "qemu/compiler.h" for compiler-specific arrangements,
> automatically included by "qemu/osdep.h" for each source file. No
> need to explicitly include a header for a Clang particularity,
> let the common "qemu/compiler.h" deal with that by having it
> include "qemu/clang-tsa.h" (renamed as qemu/clang-tsa.h.inc).
> Add a check to not include "qemu/clang-tsa.h.inc" directly,
> remove previous "qemu/clang-tsa.h" inclusions.
>
> Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v2] qemu/compiler: Absorb 'clang-tsa.h'

[Kevin Wolf]

Am 16.01.2025 um 22:11 hat Philippe Mathieu-Daudé geschrieben:
> We already have "qemu/compiler.h" for compiler-specific arrangements,
> automatically included by "qemu/osdep.h" for each source file. No
> need to explicitly include a header for a Clang particularity,
> let the common "qemu/compiler.h" deal with that by having it
> include "qemu/clang-tsa.h" (renamed as qemu/clang-tsa.h.inc).
> Add a check to not include "qemu/clang-tsa.h.inc" directly,
> remove previous "qemu/clang-tsa.h" inclusions.
> 
> Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Don't we normally try to reduce the number of header files that need to
be opened during the build instead of increasing it?

Either way, if we do want to do this, the implementation looks correct.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [PATCH v2] qemu/compiler: Absorb 'clang-tsa.h'

[Richard Henderson]

On 1/16/25 13:11, Philippe Mathieu-Daudé wrote:
> We already have "qemu/compiler.h" for compiler-specific arrangements,
> automatically included by "qemu/osdep.h" for each source file. No
> need to explicitly include a header for a Clang particularity,
> let the common "qemu/compiler.h" deal with that by having it
> include "qemu/clang-tsa.h" (renamed as qemu/clang-tsa.h.inc).
> Add a check to not include "qemu/clang-tsa.h.inc" directly,
> remove previous "qemu/clang-tsa.h" inclusions.

I think merging this into compiler.h itself would be better.


r~