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,
