Re: [PATCH 9/9] exec/address-spaces: Inline legacy functions

[Bernhard Beschow]

Am 20. September 2022 09:02:41 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>
>
>On Tue, 20 Sep 2022, Philippe Mathieu-Daudé via wrote:
>
>> On 20/9/22 01:17, Bernhard Beschow wrote:
>>> The functions just access a global pointer and perform some pointer
>>> arithmetic on top. Allow the compiler to see through this by inlining.
>> 
>> I thought about this while reviewing the previous patch, ...
>> 
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>>   include/exec/address-spaces.h | 30 ++++++++++++++++++++++++++----
>>>   softmmu/physmem.c             | 28 ----------------------------
>>>   2 files changed, 26 insertions(+), 32 deletions(-)
>>> 
>>> diff --git a/include/exec/address-spaces.h b/include/exec/address-spaces.h
>>> index b31bd8dcf0..182af27cad 100644
>>> --- a/include/exec/address-spaces.h
>>> +++ b/include/exec/address-spaces.h
>>> @@ -23,29 +23,51 @@
>>>     #ifndef CONFIG_USER_ONLY
>>>   +#include "hw/boards.h"
>> 
>> ... but I'm not a fan of including this header here. It is restricted to 
>> system emulation, but still... Let see what the others think.
>
>Had the same thought first if this would break user emulation but I don't know 
>how that works (and this include is withing !CONFIG_USER_ONLY). I've checked 
>in configure now and it seems that softmmu is enabled/disabled with system, 
>which reminded me to a previous conversation where I've suggested renaming 
>softmmu to sysemu as that better shows what it's really used for and maybe the 
>real softmmu part should be split from it but I don't remember the details. If 
>it still works with --enable-user --disable-system then maybe it's OK and only 
>confusing because of misnaming sysemu as softmmu.

I've compiled all architectures w/o any --{enable,disable}-{user,system} flags 
and I had compile errors only when putting the include outside the guard. So 
this in particular doesn't seem to be a problem.

Best regards,
Bernhard
>
>Reagrds,
>BALATON Zoltan
>
>>>   /**
>>>    * Get the root memory region.  This is a legacy function, provided for
>>>    * compatibility. Prefer using SysBusState::system_memory directly.
>>>    */
>>> -MemoryRegion *get_system_memory(void);
>>> +inline MemoryRegion *get_system_memory(void)
>>> +{
>>> +    assert(current_machine);
>>> +
>>> +    return &current_machine->main_system_bus.system_memory;
>>> +}
>>>     /**
>>>    * Get the root I/O port region.  This is a legacy function, provided for
>>>    * compatibility. Prefer using SysBusState::system_io directly.
>>>    */
>>> -MemoryRegion *get_system_io(void);
>>> +inline MemoryRegion *get_system_io(void)
>>> +{
>>> +    assert(current_machine);
>>> +
>>> +    return &current_machine->main_system_bus.system_io;
>>> +}
>>>     /**
>>>    * Get the root memory address space.  This is a legacy function, 
>>> provided for
>>>    * compatibility. Prefer using SysBusState::address_space_memory directly.
>>>    */
>>> -AddressSpace *get_address_space_memory(void);
>>> +inline AddressSpace *get_address_space_memory(void)
>>> +{
>>> +    assert(current_machine);
>>> +
>>> +    return &current_machine->main_system_bus.address_space_memory;
>>> +}
>>>     /**
>>>    * Get the root I/O port address space.  This is a legacy function, 
>>> provided
>>>    * for compatibility. Prefer using SysBusState::address_space_io directly.
>>>    */
>>> -AddressSpace *get_address_space_io(void);
>>> +inline AddressSpace *get_address_space_io(void)
>>> +{
>>> +    assert(current_machine);
>>> +
>>> +    return &current_machine->main_system_bus.address_space_io;
>>> +}
>>>     #endif
>>>   diff --git a/softmmu/physmem.c b/softmmu/physmem.c
>>> index 07e9a9171c..dce088f55c 100644
>>> --- a/softmmu/physmem.c
>>> +++ b/softmmu/physmem.c
>>> @@ -2674,34 +2674,6 @@ static void memory_map_init(SysBusState *sysbus)
>>>       address_space_init(&sysbus->address_space_io, system_io, "I/O");
>>>   }
>>>   -MemoryRegion *get_system_memory(void)
>>> -{
>>> -    assert(current_machine);
>>> -
>>> -    return &current_machine->main_system_bus.system_memory;
>>> -}
>>> -
>>> -MemoryRegion *get_system_io(void)
>>> -{
>>> -    assert(current_machine);
>>> -
>>> -    return &current_machine->main_system_bus.system_io;
>>> -}
>>> -
>>> -AddressSpace *get_address_space_memory(void)
>>> -{
>>> -    assert(current_machine);
>>> -
>>> -    return &current_machine->main_system_bus.address_space_memory;
>>> -}
>>> -
>>> -AddressSpace *get_address_space_io(void)
>>> -{
>>> -    assert(current_machine);
>>> -
>>> -    return &current_machine->main_system_bus.address_space_io;
>>> -}
>>> -
>>>   static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
>>>                                        hwaddr length)
>>>   {
>> 
>> 
>>