Re: [PATCH v3 6/9] target/loongarch: Define function loongarch_get_addr_from_tlb() non-static

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 22/4/25 09:57, bibo mao wrote:
> 
> 
> On 2025/4/22 下午3:46, Philippe Mathieu-Daudé wrote:
>> On 22/4/25 09:39, bibo mao wrote:
>>>
>>>
>>> On 2025/4/22 下午3:18, Philippe Mathieu-Daudé wrote:
>>>> On 22/4/25 04:57, Bibo Mao wrote:
>>>>> Define function loongarch_get_addr_from_tlb() non-static, and add its
>>>>> definition in header file tcg/tcg_loongarch.h
>>>>>
>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>> ---
>>>>>   target/loongarch/cpu_helper.c        | 10 ++--------
>>>>>   target/loongarch/tcg/tcg_loongarch.h | 16 ++++++++++++++++
>>>>>   2 files changed, 18 insertions(+), 8 deletions(-)


>>>>> diff --git a/target/loongarch/tcg/tcg_loongarch.h b/target/ 
>>>>> loongarch/ tcg/tcg_loongarch.h
>>>>> index da2539e995..69a93bfc3e 100644
>>>>> --- a/target/loongarch/tcg/tcg_loongarch.h
>>>>> +++ b/target/loongarch/tcg/tcg_loongarch.h
>>>>> @@ -6,7 +6,23 @@
>>>>>    */
>>>>>   #ifndef TARGET_LOONGARCH_TCG_LOONGARCH_H
>>>>>   #define TARGET_LOONGARCH_TCG_LOONGARCH_H
>>>>> +#include "cpu.h"
>>>>>   void loongarch_csr_translate_init(void);
>>>>> +#ifdef CONFIG_TCG
>>>>> +int loongarch_get_addr_from_tlb(CPULoongArchState *env, hwaddr 
>>>>> *physical,
>>>>> +                                int *prot, target_ulong address,
>>>>> +                                MMUAccessType access_type, int 
>>>>> mmu_idx);
>>>>> +#else
>>>>> +static inline int loongarch_get_addr_from_tlb(CPULoongArchState *env,
>>>>> +                                              hwaddr *physical,
>>>>> +                                              int *prot, 
>>>>> target_ulong address,
>>>>> +                                              MMUAccessType 
>>>>> access_type,
>>>>> +                                              int mmu_idx)
>>>>> +{
>>>>> +    return TLBRET_NOMATCH;
>>>>
>>>> CONFIG_TCG should always be defined when including tcg/tcg_loongarch.h.
>>>
>>> If so, there will be no stub function declaration with 
>>> loongarch_get_addr_from_tlb(). *#ifdef CONFIG_TCG* needs be added in 
>>> c files such as:
>>>
>>> static int loongarch_map_address(CPULoongArchState *env, hwaddr 
>>> *physical,
>>>                                   int *prot, target_ulong address,
>>>                                   MMUAccessType access_type, int 
>>> mmu_idx,
>>>                                   int is_debug)
>>> {
>>>      int ret;
>>>
>>> *#ifdef CONFIG_TCG*
>>>      if (!kvm_enabled()) {
>>
>> Maybe what we want here is:
>>
>>        if (tcg_enabled()) {
> yeap, it works with --enable-debug --disable-tcg option.
> 
> I had thought that loongarch_get_addr_from_tlb should be defined if this 
> file is compiled with -O0 option. It is skipped even with -O0 option.
>      if  (0) {

'if (0)' skips at *compile* time, while 'if (tcg_enabled())' would
skip at *link* time, so we still need to prototype declaration to
compile the source file, but not the stub.

>          ret = loongarch_get_addr_from_tlb(env, physical, prot, address,
>                                            access_type, mmu_idx);
>      }
> 
> And thanks for your guidance.

You are welcome! The cleanups you are doing are not super interesting
(lot of code churn) but make the code easier to follow and maintain IMO.

> 
> Regards
> Bibo Mao