[Top][All Lists]

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

Re: [PATCH v1 1/1] riscv: Pass RISCVHartArrayState by pointer

From: Eric Blake
Subject: Re: [PATCH v1 1/1] riscv: Pass RISCVHartArrayState by pointer
Date: Tue, 19 Jan 2021 15:50:17 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0

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é <f4bug@amsat.org> 
>> wrote:
>>> On 1/16/21 12:00 AM, Alistair Francis wrote:
>>>> We were accidently passing RISCVHartArrayState by value instead of


>>>> 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

reply via email to

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