Re: [RFC v17 10/14] i386: split tcg btp_helper into softmmu and user parts

[Claudio Fontana <cfontana@suse.de>]

On 2/11/21 11:39 AM, Philippe Mathieu-Daudé wrote:
> On 2/11/21 11:07 AM, Claudio Fontana wrote:
>> On 2/10/21 5:28 PM, Alex Bennée wrote:
>>>
>>> Claudio Fontana <cfontana@suse.de> writes:
>>>
>>> s/btp/bpt/ in subject line...
>>>
>>>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>>>> ---
>>>>  target/i386/tcg/helper-tcg.h                 |   3 +
>>>>  target/i386/tcg/bpt_helper.c                 | 275 -----------------
>>>>  target/i386/tcg/softmmu/bpt_helper_softmmu.c | 293 +++++++++++++++++++
>>>>  target/i386/tcg/user/bpt_helper_user.c       |  33 +++
>>>
>>> So I'm not sure about totally mirroring the file names in softmmu/user
>>> subdirs. I can see it makes sense in some cases where there are genuine
>>> functional differences between the two. However for everything that
>>> exists only for one mode we might as well throw the stubs into one file.
>>> Maybe target/tcg/user/stubs.c in this case?
>>
>>
>> Hi Alex, I think you are right, repeating the _softmmu , _user seems too much.
>>
>> On similar things in the past Paolo mentioned that he favours simpler naming.
>>
>> In this case I could do for example:
>>
>> target/i386/tcg/seg_helper.c          - seg helper common parts
>> target/i386/tcg/softmmu/seg_helper.c  - seg helper softmmu-only code
>> target/i386/tcg/user/seg_helper.c     - seg helper user-only code
> 
> What about:
> 
>   target/i386/tcg/seg_helper.c          - seg helper common parts
>   target/i386/tcg/seg_helper_softmmu.c  - seg helper softmmu-only code
>   target/i386/tcg/seg_helper_user.c     - seg helper user-only code

Hi Philippe,

I tried this one in particular, it looked a bit confusing and cluttered to me when looking at the overall result:

$ ls
bpt_helper.c
bpt_helper_softmmu.c
bpt_helper_user.c
cc_helper.c
cc_helper_softmmu.c
cc_helper_template.h
cc_helper_user.c
excp_helper.c
excp_helper_softmmu.c
excp_helper_user.c
fpu_helper.c
fpu_helper_softmmu.c
fpu_helper_user.c
helper-tcg.h
int_helper.c
int_helper_softmmu.c
int_helper_user.c
mem_helper.c
mem_helper_softmmu.c
mem_helper_user.c
meson.build
misc_helper.c
misc_helper_softmmu.c
misc_helper_user.c
mpx_helper.c
mpx_helper_softmmu.c
mpx_helper_user.c
seg_helper.c
seg_helper.h
seg_helper_softmmu.c
seg_helper_user.c
tcg-cpu.c
tcg-cpu.h
tcg-cpu-softmmu.c
tcg-cpu-user.c
tcg-stub.c
translate.c


> 
>> For the parts that are just stubs really (like here bpt for user), I would like to see if I can remove them completely if possible..
> 
> It is probably worth spend time on this first. If the helpers are not
> needed, why do we generate the code in the first place?

Worth a good look.

> 
>>
>> Overall though, I am wondering whether this kind of change (extended more to the rest of the target/ code) is an interesting approach,
>> or does it make harder to work with the *_helper code, as people have to chase down more files?
>>
>>
>> Thank you!
>>
>> Claudio

Thanks,

Claudio