Re: [Qemu-devel] [RFC PATCH 0/6] Convert softmmu-template into normal code

[Philippe Mathieu-Daudé]

On 04/20/2018 03:35 PM, Emilio G. Cota wrote:
> On Fri, Apr 20, 2018 at 16:50:39 +0100, Alex Bennée wrote:
>> Any questions/thoughts/comments?
> 
> Very nice!

Yep :)

> 
> I modified the style a little bit, namely to pass checkpatch and to
> improve grepability by having
>   foo __attribute__((flatten))
>   function(arg1, arg2, ...)
> as opposed to
>   foo __attribute__((flatten)) function(arg1,
>                                         arg2,
>                                         ...)

I thought the same while reviewing and agree.

> 
> I also changed the patches to make the series simpler: one patch
> to add softmmu.c (without compiling it yet), one patch to
> actually use softmmu.c, and a final patch to remove the template
> file.

By "without compiling" you mean it is not compilable?
That would mean not script-bisectable.

> 
> You can get my modifications from:
>   https://github.com/cota/qemu/tree/softmmu
> 
> Note that there are two stray references to softmmu_template
> in comments:
> $ git grep -i 'softmmu_template'
> accel/tcg/translate-all.c: * Called via softmmu_template.h when code areas 
> are written to with
> accel/tcg/translate-all.c: * Called by softmmu_template.h, with iothread 
> mutex not held.
> 
> I don't have time right now to test performance, but at least
> the only function I checked (helper_ret_ldub_mmu) has only grown
> in size by a little bit, from 584 bytes to 614.
> 
> The executable is now larger:

This is odd, I was expecting some shrinking...

What kind of build are doing? More such -ggdb or such -O3?

> 
> - before:
>    text    data     bss     dec     hex filename
> 9864951 2944592 4371560 17181103        10629af 
> aarch64-softmmu/qemu-system-aarch64
> 
> - after:
>    text    data     bss     dec     hex filename
> 9892095 2944592 4371560 17208247        10693b7 
> aarch64-softmmu/qemu-system-aarch64
> 
> By removing flatten we can decrease the overall size, but this might affect 
> perf:
>    text    data     bss     dec     hex filename
> 9849967 2944592 4371560 17166119        105ef27 
> aarch64-softmmu/qemu-system-aarch64
> 
> This is a slow path though so it might be worth looking into.
> 
> BTW I haven't done any testing, just compile-tested for all targets.
> 
> So overall I think this should be upgraded from RFC to a proper patch
> series; hopefully we can get this done for 2.13!
> 
> Thanks,
> 
>               Emilio
>