qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH-for-8.0] hw: Avoid using inlined functions with external


From: Philippe Mathieu-Daudé
Subject: Re: [RFC PATCH-for-8.0] hw: Avoid using inlined functions with external linkage
Date: Fri, 16 Dec 2022 22:49:06 +0100
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.5.1

On 8/12/22 17:27, Peter Maydell wrote:
On Thu, 8 Dec 2022 at 16:11, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

When using Clang ("Apple clang version 14.0.0 (clang-1400.0.29.202)")
and building with -Wall we get:

   hw/arm/smmu-common.c:173:33: warning: static function 
'smmu_hash_remove_by_asid_iova' is used in an inline function with external 
linkage [-Wstatic-in-inline]
   hw/arm/smmu-common.h:170:1: note: use 'static' to give inline function 
'smmu_iotlb_inv_iova' internal linkage
     void smmu_iotlb_inv_iova(SMMUState *s, int asid, dma_addr_t iova,
     ^
     static

None of our code base require / use inlined functions with external
linkage. Some places use internal inlining in the hot path. These
two functions are certainly not in any hot path and don't justify
any inlining.

Reported-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC: Any better justification?

I don't really understand what the warning is trying to warn
about, and googling didn't enlighten me. Does anybody understand it?

In any case, it does seem weird to define a function inline and
also have it be defined in a C file rather than as a 'static inline'
in a header file, so these are likely oversights rather than
intentional.

---
  hw/arm/smmu-common.c | 10 +++++-----
  hw/i386/x86.c        |  3 +--
  2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 78cc131926..9ac1680180 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -64,8 +64,7 @@
  /* Physical Address of PVH entry point read from kernel ELF NOTE */
  static size_t pvh_start_addr;

-inline void init_topo_info(X86CPUTopoInfo *topo_info,
-                           const X86MachineState *x86ms)
+void init_topo_info(X86CPUTopoInfo *topo_info, const X86MachineState *x86ms)
  {
      MachineState *ms = MACHINE(x86ms);

This function is not used anywhere outside this file, so we
can delete the prototype from include/hw/i386/x86.h and
make the function "static void".

Good idea.

With those changes,
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Thanks!




reply via email to

[Prev in Thread] Current Thread [Next in Thread]