Re: [PATCH v3] elf: Rewrite long RESOLVE_MAP macro to a debug friendly function

[Fangrui Song <maskray@google.com>]

On 2022-05-15, Siddhesh Poyarekar wrote:
>On 15/05/2022 04:18, Fangrui Song wrote:
>>On 2022-05-14, Nicholas Guriev wrote:
>>>On Пн, 2022-05-09 at 19:08 +0530, Siddhesh Poyarekar wrote:
>>>>Please use __always_inline instead, which adds the inline keyword as
>>>>well as __attribute__ ((inline)).  That way gcc ensures that the
>>>>function is *always* inlined.
>>>
>>>That is not necessary. My original intention was to improve the
>>>debugging experience at this point. And the standard `inline` keyword
>>>allows one to control optimizations with the -O flag.
>
>You can't build glibc with anything less than -O2, so that's a moot 
>point.  If you don't use __always_inline here then you're not only 
>changing the debugging experience, but also potentially adding a 
>performance overhead.  gdb can do well enough in most cases of inlined 
>functions, so __always_inlined should not hamper that.

Hope that the -O2 requirement can be lifted:)

The original decision might be related to guaranteed inlining and relocations in -O0
but it should be revisited nowadays... (Both FreeBSD rtld and musl rtld allow -O0).

>>Looks good to me, I think the always_inline here does not affect
>>performance, as shown in my testing with a large PIE with many
>>R_*_RELATIVE (function call overhead is larger with a non-R_*_RELATIVE).
>
>If you want to make a decision based on this then please quantify it 
>more precisely with a microbenchmark added to benchtests.  Otherwise 
>just add __always_inline because it is a more minimal change.  TBH I'd 
>prefer the former because it then adds a new microbenchmark that we 
>can measure dynamic linker changes in future but I can live with the 
>latter.

Thanks. I can add __always_inline.
(I don't think it matters but I guess I just want to avoid a debate :))