[PATCH v5 05/12] target/arm/tcg/translate-vfp.c: make compilation unit common

Pierrick Bouvier posted 12 patches 5 days, 22 hours ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH v5 05/12] target/arm/tcg/translate-vfp.c: make compilation unit common
Posted by Pierrick Bouvier 5 days, 22 hours ago
Generated decode files must be duplicated between user and system, as
they are generated in private folders per libs, and can't be included
otherwise, as meson does not give control on output folder.
Indeed, meson generator is a different approach than custom_target, and
this is a limitation by design.

They were already duplicated between arch variants anyway, so nothing
new here.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/tcg/meson.build | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/target/arm/tcg/meson.build b/target/arm/tcg/meson.build
index 3e96c77df73..46bf4a6d76b 100644
--- a/target/arm/tcg/meson.build
+++ b/target/arm/tcg/meson.build
@@ -30,7 +30,6 @@ arm_ss.add(files(
   'translate-m-nocp.c',
   'translate-mve.c',
   'translate-neon.c',
-  'translate-vfp.c',
   'm_helper.c',
   'mve_helper.c',
   'op_helper.c',
@@ -60,7 +59,10 @@ arm_common_ss.add(files(
   'crypto_helper.c',
 ))
 
-arm_common_system_ss.add(files(
+arm_common_system_ss.add(
+  decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp'),
+  decodetree.process('vfp-uncond.decode', extra_args: '--decode=disas_vfp_uncond'),
+  files(
   'cpregs-at.c',
   'debug.c',
   'hflags.c',
@@ -68,6 +70,7 @@ arm_common_system_ss.add(files(
   'psci.c',
   'tlb_helper.c',
   'tlb-insns.c',
+  'translate-vfp.c',
   'vec_helper.c',
   'vfp_helper.c',
 ))
@@ -76,6 +79,7 @@ arm_user_ss.add(files(
   'hflags.c',
   'neon_helper.c',
   'tlb_helper.c',
+  'translate-vfp.c',
   'vec_helper.c',
   'vfp_helper.c',
 ))
-- 
2.47.3


Re: [PATCH v5 05/12] target/arm/tcg/translate-vfp.c: make compilation unit common
Posted by Richard Henderson 3 days, 14 hours ago
On 3/28/26 02:50, Pierrick Bouvier wrote:
> Generated decode files must be duplicated between user and system, as
> they are generated in private folders per libs, and can't be included
> otherwise, as meson does not give control on output folder.
> Indeed, meson generator is a different approach than custom_target, and
> this is a limitation by design.
> 
> They were already duplicated between arch variants anyway, so nothing
> new here.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   target/arm/tcg/meson.build | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/tcg/meson.build b/target/arm/tcg/meson.build
> index 3e96c77df73..46bf4a6d76b 100644
> --- a/target/arm/tcg/meson.build
> +++ b/target/arm/tcg/meson.build
> @@ -30,7 +30,6 @@ arm_ss.add(files(
>     'translate-m-nocp.c',
>     'translate-mve.c',
>     'translate-neon.c',
> -  'translate-vfp.c',
>     'm_helper.c',
>     'mve_helper.c',
>     'op_helper.c',
> @@ -60,7 +59,10 @@ arm_common_ss.add(files(
>     'crypto_helper.c',
>   ))
>   
> -arm_common_system_ss.add(files(
> +arm_common_system_ss.add(
> +  decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp'),
> +  decodetree.process('vfp-uncond.decode', extra_args: '--decode=disas_vfp_uncond'),

How are you not removing the other decodetree invocations at the top of the file?


r~

> +  files(
>     'cpregs-at.c',
>     'debug.c',
>     'hflags.c',
> @@ -68,6 +70,7 @@ arm_common_system_ss.add(files(
>     'psci.c',
>     'tlb_helper.c',
>     'tlb-insns.c',
> +  'translate-vfp.c',
>     'vec_helper.c',
>     'vfp_helper.c',
>   ))
> @@ -76,6 +79,7 @@ arm_user_ss.add(files(
>     'hflags.c',
>     'neon_helper.c',
>     'tlb_helper.c',
> +  'translate-vfp.c',
>     'vec_helper.c',
>     'vfp_helper.c',
>   ))


Re: [PATCH v5 05/12] target/arm/tcg/translate-vfp.c: make compilation unit common
Posted by Pierrick Bouvier 3 days, 13 hours ago
On 3/29/26 6:09 PM, Richard Henderson wrote:
> On 3/28/26 02:50, Pierrick Bouvier wrote:
>> Generated decode files must be duplicated between user and system, as
>> they are generated in private folders per libs, and can't be included
>> otherwise, as meson does not give control on output folder.
>> Indeed, meson generator is a different approach than custom_target, and
>> this is a limitation by design.
>>
>> They were already duplicated between arch variants anyway, so nothing
>> new here.
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    target/arm/tcg/meson.build | 8 ++++++--
>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/arm/tcg/meson.build b/target/arm/tcg/meson.build
>> index 3e96c77df73..46bf4a6d76b 100644
>> --- a/target/arm/tcg/meson.build
>> +++ b/target/arm/tcg/meson.build
>> @@ -30,7 +30,6 @@ arm_ss.add(files(
>>      'translate-m-nocp.c',
>>      'translate-mve.c',
>>      'translate-neon.c',
>> -  'translate-vfp.c',
>>      'm_helper.c',
>>      'mve_helper.c',
>>      'op_helper.c',
>> @@ -60,7 +59,10 @@ arm_common_ss.add(files(
>>      'crypto_helper.c',
>>    ))
>>    
>> -arm_common_system_ss.add(files(
>> +arm_common_system_ss.add(
>> +  decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp'),
>> +  decodetree.process('vfp-uncond.decode', extra_args: '--decode=disas_vfp_uncond'),
> 
> How are you not removing the other decodetree invocations at the top of the file?
>

They are still needed for user binaries.
As explained in commit message, we don't have control in which folder 
those .c.inc are produced, so we have to duplicate them to make them 
visible for system_common code. Since they are not proper compilation 
units but templated C, we don't really have a choice here.

The only alternative, if you prefer it, is to remove them from gen_a32 
array, and add them to arm_user_ss. In all cases, duplication is still 
needed.

Pierrick

Re: [PATCH v5 05/12] target/arm/tcg/translate-vfp.c: make compilation unit common
Posted by Richard Henderson 3 days, 11 hours ago
On 3/30/26 12:34, Pierrick Bouvier wrote:
>>> -arm_common_system_ss.add(files(
>>> +arm_common_system_ss.add(
>>> +  decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp'),
>>> +  decodetree.process('vfp-uncond.decode', extra_args: '--decode=disas_vfp_uncond'),
>>
>> How are you not removing the other decodetree invocations at the top of the file?
>>
> 
> They are still needed for user binaries.
> As explained in commit message, we don't have control in which folder those .c.inc are 
> produced, so we have to duplicate them to make them visible for system_common code. Since 
> they are not proper compilation units but templated C, we don't really have a choice here.
> 
> The only alternative, if you prefer it, is to remove them from gen_a32 array, and add them 
> to arm_user_ss. In all cases, duplication is still needed.

Maybe I'm wrong about how meson works, but does invoking .process() twice result in two 
executions of decodetree, two different file objects, possibly with identical names?

If you want to share these, do we need one invocation, storing the file object in a local 
variable?  Akin to

vfp_d = decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp')

gen_a32 = [
   ...
   vfp_d,
   ...
]

arm_common_system_ss.add(
   vfp_d,
   ...
)


r~

Re: [PATCH v5 05/12] target/arm/tcg/translate-vfp.c: make compilation unit common
Posted by Pierrick Bouvier 2 days, 23 hours ago
On 3/29/26 8:46 PM, Richard Henderson wrote:
> On 3/30/26 12:34, Pierrick Bouvier wrote:
>>>> -arm_common_system_ss.add(files(
>>>> +arm_common_system_ss.add(
>>>> +  decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp'),
>>>> +  decodetree.process('vfp-uncond.decode', extra_args: '--decode=disas_vfp_uncond'),
>>>
>>> How are you not removing the other decodetree invocations at the top of the file?
>>>
>>
>> They are still needed for user binaries.
>> As explained in commit message, we don't have control in which folder those .c.inc are
>> produced, so we have to duplicate them to make them visible for system_common code. Since
>> they are not proper compilation units but templated C, we don't really have a choice here.
>>
>> The only alternative, if you prefer it, is to remove them from gen_a32 array, and add them
>> to arm_user_ss. In all cases, duplication is still needed.
> 
> Maybe I'm wrong about how meson works, but does invoking .process() twice result in two
> executions of decodetree, two different file objects, possibly with identical names?
>

Don't forget we don't produce file objects (that's the issue here), but 
.c.inc that gets included later.

> If you want to share these, do we need one invocation, storing the file object in a local
> variable?  Akin to
> 
> vfp_d = decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp')
> 
> gen_a32 = [
>     ...
>     vfp_d,
>     ...
> ]
> 
> arm_common_system_ss.add(
>     vfp_d,
>     ...
> )
>

For information, it's already duplicated today, since it's generated per 
target.

> 
> r~


Re: [PATCH v5 05/12] target/arm/tcg/translate-vfp.c: make compilation unit common
Posted by Pierrick Bouvier 3 days ago
On 3/29/26 8:46 PM, Richard Henderson wrote:
> On 3/30/26 12:34, Pierrick Bouvier wrote:
>>>> -arm_common_system_ss.add(files(
>>>> +arm_common_system_ss.add(
>>>> +  decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp'),
>>>> +  decodetree.process('vfp-uncond.decode', extra_args: '--decode=disas_vfp_uncond'),
>>>
>>> How are you not removing the other decodetree invocations at the top of the file?
>>>
>>
>> They are still needed for user binaries.
>> As explained in commit message, we don't have control in which folder those .c.inc are
>> produced, so we have to duplicate them to make them visible for system_common code. Since
>> they are not proper compilation units but templated C, we don't really have a choice here.
>>
>> The only alternative, if you prefer it, is to remove them from gen_a32 array, and add them
>> to arm_user_ss. In all cases, duplication is still needed.
> 
> Maybe I'm wrong about how meson works, but does invoking .process() twice result in two
> executions of decodetree, two different file objects, possibly with identical names?
> 
> If you want to share these, do we need one invocation, storing the file object in a local
> variable?  Akin to
> 
> vfp_d = decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp')
> 
> gen_a32 = [
>     ...
>     vfp_d,
>     ...
> ]
> 
> arm_common_system_ss.add(
>     vfp_d,
>     ...
> )
> 
> 
> r~

Yes, your idea works. It will still result in duplicated .c.inc though.

We can't have a single invocation of the generator, since the result 
will be private to a specific binary/library.

You can see it by inspecting build.ninja directly:

$ grep 'decodetree.py.*decode-vfp.c.inc' build/build.ninja
... -o libsystem_arm.a.p/decode-vfp.c.inc
... -o libqemu-aarch64-linux-user.a.p/decode-vfp.c.inc
...

As you see, the output folder is releated to the final library (or 
executable) that will include it, and not to where we are in terms in 
source folder.
build/libsystem_arm.a.p is only included for common-system-arm code 
while libqemu-aarch64-linux-user.a.p is only visible for arm-user code.

Pierrick

Re: [PATCH v5 05/12] target/arm/tcg/translate-vfp.c: make compilation unit common
Posted by Richard Henderson 2 days, 10 hours ago
On 3/31/26 01:38, Pierrick Bouvier wrote:
>> If you want to share these, do we need one invocation, storing the file object in a local
>> variable?  Akin to
>>
>> vfp_d = decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp')
>>
>> gen_a32 = [
>>     ...
>>     vfp_d,
>>     ...
>> ]
>>
>> arm_common_system_ss.add(
>>     vfp_d,
>>     ...
>> )
>>
>>
>> r~
> 
> Yes, your idea works. It will still result in duplicated .c.inc though.
> 
> We can't have a single invocation of the generator, since the result will be private to a 
> specific binary/library.
> 
> You can see it by inspecting build.ninja directly:
> 
> $ grep 'decodetree.py.*decode-vfp.c.inc' build/build.ninja
> ... -o libsystem_arm.a.p/decode-vfp.c.inc
> ... -o libqemu-aarch64-linux-user.a.p/decode-vfp.c.inc
> ...
> 
> As you see, the output folder is releated to the final library (or executable) that will 
> include it, and not to where we are in terms in source folder.
> build/libsystem_arm.a.p is only included for common-system-arm code while libqemu-aarch64- 
> linux-user.a.p is only visible for arm-user code.

Before:

$ fgrep decode-vfp.c.inc buildlog
[2638/6697] Generating 'libqemu-aarch64-linux-user.a.p/decode-vfp.c.inc'
[2719/6697] Generating 'libqemu-aarch64_be-linux-user.a.p/decode-vfp.c.inc'
[2830/6697] Generating 'libqemu-arm-linux-user.a.p/decode-vfp.c.inc'
[2904/6697] Generating 'libqemu-armeb-linux-user.a.p/decode-vfp.c.inc'
[4070/6697] Generating 'libqemu-aarch64-softmmu.a.p/decode-vfp.c.inc'
[4174/6697] Generating 'libqemu-arm-softmmu.a.p/decode-vfp.c.inc'

After:

$ fgrep decode-vfp.c.inc buildlog
[2504/6701] Generating 'libsystem_arm.a.p/decode-vfp.c.inc'
[2651/6701] Generating 'libqemu-aarch64-linux-user.a.p/decode-vfp.c.inc'
[2732/6701] Generating 'libqemu-aarch64_be-linux-user.a.p/decode-vfp.c.inc'
[2843/6701] Generating 'libqemu-arm-linux-user.a.p/decode-vfp.c.inc'
[2918/6701] Generating 'libqemu-armeb-linux-user.a.p/decode-vfp.c.inc'
[4085/6701] Generating 'libqemu-aarch64-softmmu.a.p/decode-vfp.c.inc'
[4183/6701] Generating 'libqemu-arm-softmmu.a.p/decode-vfp.c.inc'

You're still building in libqemu-{arm,aarch64}-softmmu.a.p.
Which you say above you aren't.


r~

Re: [PATCH v5 05/12] target/arm/tcg/translate-vfp.c: make compilation unit common
Posted by Pierrick Bouvier 1 day, 20 hours ago
On 3/30/26 10:20 PM, Richard Henderson wrote:
> On 3/31/26 01:38, Pierrick Bouvier wrote:
>>> If you want to share these, do we need one invocation, storing the file object in a local
>>> variable?  Akin to
>>>
>>> vfp_d = decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp')
>>>
>>> gen_a32 = [
>>>      ...
>>>      vfp_d,
>>>      ...
>>> ]
>>>
>>> arm_common_system_ss.add(
>>>      vfp_d,
>>>      ...
>>> )
>>>
>>>
>>> r~
>>
>> Yes, your idea works. It will still result in duplicated .c.inc though.
>>
>> We can't have a single invocation of the generator, since the result will be private to a
>> specific binary/library.
>>
>> You can see it by inspecting build.ninja directly:
>>
>> $ grep 'decodetree.py.*decode-vfp.c.inc' build/build.ninja
>> ... -o libsystem_arm.a.p/decode-vfp.c.inc
>> ... -o libqemu-aarch64-linux-user.a.p/decode-vfp.c.inc
>> ...
>>
>> As you see, the output folder is releated to the final library (or executable) that will
>> include it, and not to where we are in terms in source folder.
>> build/libsystem_arm.a.p is only included for common-system-arm code while libqemu-aarch64-
>> linux-user.a.p is only visible for arm-user code.
> 
> Before:
> 
> $ fgrep decode-vfp.c.inc buildlog
> [2638/6697] Generating 'libqemu-aarch64-linux-user.a.p/decode-vfp.c.inc'
> [2719/6697] Generating 'libqemu-aarch64_be-linux-user.a.p/decode-vfp.c.inc'
> [2830/6697] Generating 'libqemu-arm-linux-user.a.p/decode-vfp.c.inc'
> [2904/6697] Generating 'libqemu-armeb-linux-user.a.p/decode-vfp.c.inc'
> [4070/6697] Generating 'libqemu-aarch64-softmmu.a.p/decode-vfp.c.inc'
> [4174/6697] Generating 'libqemu-arm-softmmu.a.p/decode-vfp.c.inc'
> 
> After:
> 
> $ fgrep decode-vfp.c.inc buildlog
> [2504/6701] Generating 'libsystem_arm.a.p/decode-vfp.c.inc'
> [2651/6701] Generating 'libqemu-aarch64-linux-user.a.p/decode-vfp.c.inc'
> [2732/6701] Generating 'libqemu-aarch64_be-linux-user.a.p/decode-vfp.c.inc'
> [2843/6701] Generating 'libqemu-arm-linux-user.a.p/decode-vfp.c.inc'
> [2918/6701] Generating 'libqemu-armeb-linux-user.a.p/decode-vfp.c.inc'
> [4085/6701] Generating 'libqemu-aarch64-softmmu.a.p/decode-vfp.c.inc'
> [4183/6701] Generating 'libqemu-arm-softmmu.a.p/decode-vfp.c.inc'
> 
> You're still building in libqemu-{arm,aarch64}-softmmu.a.p.
> Which you say above you aren't.
>

Even though the libqemu.*softmmu are generated, they are not used since 
libsystem_arm is included instead. This is because gen_a32 is included 
in all targets.

I'll go with your suggestion to have a variable per decode file, and 
include it in system and user targets only, which will reduce 
duplication to the minimum we can.

> 
> r~