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

[Jerome Forissier]

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,
-- 
Jerome