diff mbox series

[v5,41/48] target/arm/tcg/crypto_helper: compile file twice (system, user)

Message ID 20250505015223.3895275-42-pierrick.bouvier@linaro.org
State Superseded
Headers show
Series single-binary: compile target/arm twice | expand

Commit Message

Pierrick Bouvier May 5, 2025, 1:52 a.m. UTC
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/tcg/crypto_helper.c | 4 +++-
 target/arm/tcg/meson.build     | 8 +++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Richard Henderson May 5, 2025, 6:38 p.m. UTC | #1
On 5/4/25 18:52, Pierrick Bouvier wrote:
> --- a/target/arm/tcg/meson.build
> +++ b/target/arm/tcg/meson.build
> @@ -30,7 +30,6 @@ arm_ss.add(files(
>     'translate-mve.c',
>     'translate-neon.c',
>     'translate-vfp.c',
> -  'crypto_helper.c',
>     'hflags.c',
>     'iwmmxt_helper.c',
>     'm_helper.c',
> @@ -63,3 +62,10 @@ arm_system_ss.add(files(
>   
>   arm_system_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('cpu-v7m.c'))
>   arm_user_ss.add(when: 'TARGET_AARCH64', if_false: files('cpu-v7m.c'))
> +
> +arm_common_system_ss.add(files(
> +  'crypto_helper.c',
> +))
> +arm_user_ss.add(files(
> +  'crypto_helper.c',
> +))

Could this use arm_common_ss?  I don't see anything that needs to be built user/system in 
this file...


r~
Pierrick Bouvier May 5, 2025, 6:47 p.m. UTC | #2
On 5/5/25 11:38 AM, Richard Henderson wrote:
> On 5/4/25 18:52, Pierrick Bouvier wrote:
>> --- a/target/arm/tcg/meson.build
>> +++ b/target/arm/tcg/meson.build
>> @@ -30,7 +30,6 @@ arm_ss.add(files(
>>      'translate-mve.c',
>>      'translate-neon.c',
>>      'translate-vfp.c',
>> -  'crypto_helper.c',
>>      'hflags.c',
>>      'iwmmxt_helper.c',
>>      'm_helper.c',
>> @@ -63,3 +62,10 @@ arm_system_ss.add(files(
>>    
>>    arm_system_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('cpu-v7m.c'))
>>    arm_user_ss.add(when: 'TARGET_AARCH64', if_false: files('cpu-v7m.c'))
>> +
>> +arm_common_system_ss.add(files(
>> +  'crypto_helper.c',
>> +))
>> +arm_user_ss.add(files(
>> +  'crypto_helper.c',
>> +))
> 
> Could this use arm_common_ss?  I don't see anything that needs to be built user/system in
> this file...
> 

It needs vec_internal.h (clear_tail), which needs CPUARMState, which 
pulls cpu.h, which uses CONFIG_USER_ONLY.

I'll take a look to break this dependency, so it can be built only once, 
and for other files as well.

> 
> r~
Richard Henderson May 5, 2025, 6:51 p.m. UTC | #3
On 5/5/25 11:47, Pierrick Bouvier wrote:
> On 5/5/25 11:38 AM, Richard Henderson wrote:
>> On 5/4/25 18:52, Pierrick Bouvier wrote:
>>> --- a/target/arm/tcg/meson.build
>>> +++ b/target/arm/tcg/meson.build
>>> @@ -30,7 +30,6 @@ arm_ss.add(files(
>>>      'translate-mve.c',
>>>      'translate-neon.c',
>>>      'translate-vfp.c',
>>> -  'crypto_helper.c',
>>>      'hflags.c',
>>>      'iwmmxt_helper.c',
>>>      'm_helper.c',
>>> @@ -63,3 +62,10 @@ arm_system_ss.add(files(
>>>    arm_system_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('cpu-v7m.c'))
>>>    arm_user_ss.add(when: 'TARGET_AARCH64', if_false: files('cpu-v7m.c'))
>>> +
>>> +arm_common_system_ss.add(files(
>>> +  'crypto_helper.c',
>>> +))
>>> +arm_user_ss.add(files(
>>> +  'crypto_helper.c',
>>> +))
>>
>> Could this use arm_common_ss?  I don't see anything that needs to be built user/system in
>> this file...
>>
> 
> It needs vec_internal.h (clear_tail), which needs CPUARMState, which pulls cpu.h, which 
> uses CONFIG_USER_ONLY.

Ah, right.  I didn't see that coming.  :-)

> I'll take a look to break this dependency, so it can be built only once, and for other 
> files as well.

Thanks.  Building twice is still an improvement, so for this set,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Pierrick Bouvier May 5, 2025, 6:53 p.m. UTC | #4
On 5/5/25 11:51 AM, Richard Henderson wrote:
> On 5/5/25 11:47, Pierrick Bouvier wrote:
>> On 5/5/25 11:38 AM, Richard Henderson wrote:
>>> On 5/4/25 18:52, Pierrick Bouvier wrote:
>>>> --- a/target/arm/tcg/meson.build
>>>> +++ b/target/arm/tcg/meson.build
>>>> @@ -30,7 +30,6 @@ arm_ss.add(files(
>>>>       'translate-mve.c',
>>>>       'translate-neon.c',
>>>>       'translate-vfp.c',
>>>> -  'crypto_helper.c',
>>>>       'hflags.c',
>>>>       'iwmmxt_helper.c',
>>>>       'm_helper.c',
>>>> @@ -63,3 +62,10 @@ arm_system_ss.add(files(
>>>>     arm_system_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('cpu-v7m.c'))
>>>>     arm_user_ss.add(when: 'TARGET_AARCH64', if_false: files('cpu-v7m.c'))
>>>> +
>>>> +arm_common_system_ss.add(files(
>>>> +  'crypto_helper.c',
>>>> +))
>>>> +arm_user_ss.add(files(
>>>> +  'crypto_helper.c',
>>>> +))
>>>
>>> Could this use arm_common_ss?  I don't see anything that needs to be built user/system in
>>> this file...
>>>
>>
>> It needs vec_internal.h (clear_tail), which needs CPUARMState, which pulls cpu.h, which
>> uses CONFIG_USER_ONLY.
> 
> Ah, right.  I didn't see that coming.  :-)
>

I like the idea to have it built once though, since so far 
{arch}_common_ss was not used, and I was not even sure such a 
compilation unit exists.

>> I'll take a look to break this dependency, so it can be built only once, and for other
>> files as well.
> 
> Thanks.  Building twice is still an improvement, so for this set,
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> r~
Pierrick Bouvier May 5, 2025, 7:17 p.m. UTC | #5
On 5/5/25 11:53 AM, Pierrick Bouvier wrote:
> On 5/5/25 11:51 AM, Richard Henderson wrote:
>> On 5/5/25 11:47, Pierrick Bouvier wrote:
>>> On 5/5/25 11:38 AM, Richard Henderson wrote:
>>>> On 5/4/25 18:52, Pierrick Bouvier wrote:
>>>>> --- a/target/arm/tcg/meson.build
>>>>> +++ b/target/arm/tcg/meson.build
>>>>> @@ -30,7 +30,6 @@ arm_ss.add(files(
>>>>>        'translate-mve.c',
>>>>>        'translate-neon.c',
>>>>>        'translate-vfp.c',
>>>>> -  'crypto_helper.c',
>>>>>        'hflags.c',
>>>>>        'iwmmxt_helper.c',
>>>>>        'm_helper.c',
>>>>> @@ -63,3 +62,10 @@ arm_system_ss.add(files(
>>>>>      arm_system_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('cpu-v7m.c'))
>>>>>      arm_user_ss.add(when: 'TARGET_AARCH64', if_false: files('cpu-v7m.c'))
>>>>> +
>>>>> +arm_common_system_ss.add(files(
>>>>> +  'crypto_helper.c',
>>>>> +))
>>>>> +arm_user_ss.add(files(
>>>>> +  'crypto_helper.c',
>>>>> +))
>>>>
>>>> Could this use arm_common_ss?  I don't see anything that needs to be built user/system in
>>>> this file...
>>>>
>>>
>>> It needs vec_internal.h (clear_tail), which needs CPUARMState, which pulls cpu.h, which
>>> uses CONFIG_USER_ONLY.
>>
>> Ah, right.  I didn't see that coming.  :-)
>>
> 
> I like the idea to have it built once though, since so far
> {arch}_common_ss was not used, and I was not even sure such a
> compilation unit exists.
>

Done.

>>> I'll take a look to break this dependency, so it can be built only once, and for other
>>> files as well.
>>
>> Thanks.  Building twice is still an improvement, so for this set,
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>
>> r~
>
diff mbox series

Patch

diff --git a/target/arm/tcg/crypto_helper.c b/target/arm/tcg/crypto_helper.c
index 7cadd61e124..ca14bd17a58 100644
--- a/target/arm/tcg/crypto_helper.c
+++ b/target/arm/tcg/crypto_helper.c
@@ -12,12 +12,14 @@ 
 #include "qemu/osdep.h"
 
 #include "cpu.h"
-#include "exec/helper-proto.h"
 #include "tcg/tcg-gvec-desc.h"
 #include "crypto/aes-round.h"
 #include "crypto/sm4.h"
 #include "vec_internal.h"
 
+#define HELPER_H "tcg/helper.h"
+#include "exec/helper-proto.h.inc"
+
 union CRYPTO_STATE {
     uint8_t    bytes[16];
     uint32_t   words[4];
diff --git a/target/arm/tcg/meson.build b/target/arm/tcg/meson.build
index dd12ccedb18..e3be0eb22b2 100644
--- a/target/arm/tcg/meson.build
+++ b/target/arm/tcg/meson.build
@@ -30,7 +30,6 @@  arm_ss.add(files(
   'translate-mve.c',
   'translate-neon.c',
   'translate-vfp.c',
-  'crypto_helper.c',
   'hflags.c',
   'iwmmxt_helper.c',
   'm_helper.c',
@@ -63,3 +62,10 @@  arm_system_ss.add(files(
 
 arm_system_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('cpu-v7m.c'))
 arm_user_ss.add(when: 'TARGET_AARCH64', if_false: files('cpu-v7m.c'))
+
+arm_common_system_ss.add(files(
+  'crypto_helper.c',
+))
+arm_user_ss.add(files(
+  'crypto_helper.c',
+))