Re: [PATCH 3/6] accel/tcg: Restrict tb_gen_code() from other accelerators

[Philippe Mathieu-Daudé]

On 3/15/21 2:52 PM, Claudio Fontana wrote:
> On 1/21/21 7:06 AM, Richard Henderson wrote:
>> On 1/17/21 11:12 PM, Claudio Fontana wrote:
>>> On 1/17/21 5:48 PM, Philippe Mathieu-Daudé wrote:
>>>> tb_gen_code() is only called within TCG accelerator,
>>>> declare it locally.
>>>
>>> Is this used only in accel/tcg/cpu-exec.c ? Should it be a static function 
>>> there?
>>
>> Possibly, but there's a *lot* of code that would have to be moved.  For now,
>> queuing a slightly modified version of the patch.
>>
>>>> --- a/accel/tcg/user-exec.c
>>>> +++ b/accel/tcg/user-exec.c
>>>> @@ -28,6 +28,7 @@
>>>>  #include "qemu/atomic128.h"
>>>>  #include "trace/trace-root.h"
>>>>  #include "trace/mem.h"
>>>> +#include "internal.h"
>>
>> Not needed by this patch.
>>
>>
>> r~
>>
> 
> Hello,
> 
> resurrecting this, and in reference to its commit: 
> "c03f041f128301c6a6c32242846be08719cd4fc3",
> 
> the name "internal.h" ends up polluting the include paths,
> so that when working for example on s390x, including "internal.h" ends up 
> including this instead of the file in target/s390x/.
> 
> I am not sure what exactly the right solution is, for this specific problem,
> and if we should look at the include paths settings in detail,
> 
> but in my view calling files just "internal.h" or "internals.h" in general is 
> not a good idea.
> 
> I can see two issues with this naming:
> 
> 1) it describes nothing about the actual intended contents, other that they 
> should be "internal".
> Rather it would be better to know what the file is intended to contain, or we 
> end up (as we end up) with very large files containing completely unrelated 
> content.
> 
> 2) we end up with clashes in our include paths if we are not super careful.
> 
> Probably in this case, the target/s390x/internal.h could be given another 
> name (s390x-internal.h) and then split up in the future (there is a whole 
> bunch of unrelated suff).
> 
> For accel/tcg/internal.h, maybe renaming it, or removing it altogether could 
> both be good options?

I think something like commit ed5bad39e57 is required, restricting
the scope of:

  add_project_arguments('-iquote',
                        meson.current_source_dir() / 'tcg' / tcg_arch,

... to tcg, and ...
                        '-iquote',
                        meson.current_source_dir() / 'accel/tcg',

to accel.

                        language: ['c', 'cpp', 'objc'])