On 1/17/21 10:52 AM, Philippe Mathieu-Daudé wrote:
> On 1/16/21 11:38 PM, Alistair Francis wrote:
>> On Sat, Jan 16, 2021 at 2:32 PM Philippe Mathieu-Daudé <f4...@amsat.org>
>> wrote:
>>>
>>> On 1/16/21 12:00 AM, Alistair Francis wrote:
>>>> We were accidently passing RISCVHartArrayState by value instead of
accidentally
>>>> pointer. The type is 824 bytes long so let's correct that and pass it by
>>>> pointer instead.
>>>> -bool riscv_is_32bit(RISCVHartArrayState harts)
>>>> +bool riscv_is_32bit(RISCVHartArrayState *harts)
Definitely better,
>>>> {
>>>> - RISCVCPU hart = harts.harts[0];
>>>> + RISCVCPU hart = harts->harts[0];
but yeah, this still results in a copy (unless the compiler optimizes it).
>>>
>>> This doesn't look improved. Maybe you want:
>>>
>>> return riscv_cpu_is_32bit(&harts->harts[0].env);
Whereas this is obviously a pointer into the original without relying on
the compiler to elide a copy.
>>
>> I suspect this ends up generating the same code.
>
> If the compiler is smart enough, but I'm not sure it can figure out
> only 1 element from the structure is accessed...
> My understanding is "first copy the content pointed at '*harts' in
> 'hart' on the stack", then only use "env".
>
> Cc'ing Eric/Richard to double check.
I agree that relying on the compiler optimization is not as
straightforward as writing the code to directly access the correct
pointer from the get-go.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
