[PATCH v2 2/3] common: board: make initcalls static

[Jerome Forissier]

Gentle ping. Can we keep the macro?

Thanks,
-- 
Jerome

On 1/27/25 11:32, Jerome Forissier wrote:
> 
> 
> On 1/25/25 00:58, Marek Vasut wrote:
>> On 1/24/25 6:23 PM, Jerome Forissier wrote:
>>>
>>>
>>> On 1/24/25 17:23, Marek Vasut wrote:
>>>> On 1/24/25 4:57 PM, Jerome Forissier wrote:
>>>>>
>>>>>
>>>>> On 1/24/25 11:35, Marek Vasut wrote:
>>>>>> On 1/24/25 10:10 AM, Jerome Forissier wrote:
>>>>>>> +#define INITCALL(_call) \
>>>>>>> +    do { \
>>>>>>> +        if (_call()) { \
>>>>>>> +            printf("%s(): initcall %s() failed\n", __func__, \
>>>>>>> +                   #_call); \
>>>>>>> +            hang(); \
>>>>>>> +        } \
>>>>>>> +    } while (0)
>>>>>>
>>>>>> Can this be turned into some static inline function too , so typechecking would be retained ? Maybe the function can be passed in a function pointer to call ?
>>>>>
>>>>>
>>>>> Doing the below totally kills the space gain (-160 bytes instead of -2281
>>>>> on zynqmp_kria_defconfig, with LTO).
>>>>
>>>> Does the compiler not inline the functions perhaps ?
>>>
>>> It does. inline vs __always_inline makes no difference. The assembly
>>> code is pretty difficult to understand in any case (macro or static
>>> inline function). There are pieces of called functions all over the
>>> place inside initcall_run_f() which is to be expected with LTO I
>>> suppose.
>>>
>>>>
>>>>> And it prevents from printing the
>>>>> function name in case of error, which is nicer than an address
>>>>> (especially with relocation at play).
>>>> That function name can be passed in using __func__ as a parameter.
>>>
>>> True. That being said, the type-checking argument does not seem
>>> decisive here, and although I too prefer to use static inline
>>> functions over macros when possible, it looks like we have no choice
>>> in this case.
>> Can you try this kind of thing (might need some tweaking):
>>
>> "
>> static void check_and_fail(int *ret) {
>>   if (*ret) {
>>     printf("failed\n");
>>     hang();
>>   }
>> }
>>
>> static inline void INITCALL(int (*_call)(void))
>> {
>>   int ret __attribute__((__cleanup__(check_and_fail))) = _call();
>> }
>> "
>>
>> (maybe it really is the passing of pointer to _call() function into the INITCALL() function which confuses gcc into not inlining this right, but let's find out)
> 
> It seems to be working as expected (size-wise). Comparing the macro
> version with yours, without LTO then with LTO:
> 
> $ ../linux/scripts/bloat-o-meter /tmp/u-boot.macro /tmp/u-boot.inlinefn
> add/remove: 2/0 grow/shrink: 1/2 up/down: 1386/-1700 (-314)
> Function                                     old     new   delta
> initcall_run_f                                 -    1316   +1316
> check_and_fail.isra                            -      64     +64
> version_string                                70      76      +6
> board_init_r                                 836     664    -172
> board_init_f                                1572      44   -1528
> Total: Before=1121213, After=1120899, chg -0.03%
> 
> $ ../linux/scripts/bloat-o-meter /tmp/u-boot.macro.lto /tmp/u-boot.inlinefn.lto
> add/remove: 5/3 grow/shrink: 2/3 up/down: 11418/-11072 (346)
> Function                                     old     new   delta
> initcall_run_r                                 -    7532   +7532
> initcall_run_f                                 -    3540   +3540
> add_to_list.lto_priv                           -     248    +248
> zynqmp_power                                   -      48     +48
> check_and_fail.isra                            -      32     +32
> __func__                                     975     987     +12
> version_string                                70      76      +6
> __func__.lto_priv                           3700    3688     -12
> zynqmp_power.lto_priv                         48       -     -48
> add_to_list                                  248       -    -248
> get_mem_top                                  568       -    -568
> board_init_f                                2880      56   -2824
> board_init_r                                7424      52   -7372
> Total: Before=1068190, After=1068536, chg +0.03%
> 
> But note that the two are not equivalent since yours doesn't print
> the initcall name in case of failure.
> 
> Regards,