[PATCH 4/6] arm: Use the WEAK assembly entry point consistently

[Tom Rini]

On Thu, Nov 24, 2022 at 12:40:44AM +0100, Pali Rohár wrote:
> On Wednesday 23 November 2022 18:15:17 Tom Rini wrote:
> > On Wed, Nov 23, 2022 at 11:50:59PM +0100, Pali Rohár wrote:
> > > On Tuesday 22 November 2022 12:31:56 Tom Rini wrote:
> > > > It is a bad idea, and more modern toolchains will fail, if you declare
> > > > an assembly function to be global and then weak, instead of declaring it
> > > > weak to start with. Update assorted assembly files to use the WEAK macro
> > > > directly.
> > > > 
> > > > Signed-off-by: Tom Rini <trini at konsulko.com>
> > > 
> > > During debugging of Nokia N900 code I was looking at this and I
> > > originally thought that this redefinition is the issue why N900 u-boot
> > > did not work... (but as we now know, the n900 issue was somewhere else).
> > > 
> > > So I agree with this change, feel free to add my:
> > > 
> > > Reviewed-by: Pali Rohár <pali at kernel.org>
> > > 
> > > ... but even after this change, linked u-boot.bin binary is
> > > not-so-correct. It works but has an issue: In final u-boot.bin binary
> > > there is both weak and non-weak version of every weak function. You can
> > > verify it for example by looking at "save_boot_params" code (really
> > > code, not just symbol) in u-boot ELF binary.
> > > 
> > > The reason for this is that linker (even LTO enabled) cannot eliminate
> > > code for weak version of function because it does not know how to
> > > "drop" it from binary/assembly code. So linker just set that non-weak
> > > version of function is active and let non-weak version present in binary
> > > as probably dead code.
> > > 
> > > This is affected only by assembly files, not by C files, because gcc is
> > > called with -ffunction-sections -fdata-sections flags which cause that
> > > every (weak) function is in its separate section (so C function
> > > "int abc() { ... }" is put into the section ".text.abc" instead of
> > > ".text") and linker's flag --gc-sections (or LTO optimization) then drop
> > > all unreferenced sections.
> > > 
> > > I do not know how fix this issue in assembly files. But cannot be WEAK
> > > macro modified to change section to ".text.<entry_name>" to mimic C
> > > compiler behavior? Would this cause any issues?
> > 
> > Yes, you're right about the cause, and potential solution. If you can
> > come up with a way to get each assembly function put in to a separate
> > .text.funcname section, that would be great and much appreciated. I
> > think I tried this at one point a long long time ago and it did work,
> > but I didn't have something clean, either. I think I was hoping that the
> > linux kernel folks would solve it in time, but they decided the
> > time/effort for --gc-sections wasn't worth it, in the end.
> 
> I quickly looked at this. If "as" is invoked with --sectname-subst flag
> then it is possible to use '.section %S.<func_name>' and '.previous'
> directives. See documentation where is example of that:
> https://sourceware.org/binutils/docs/as/Section.html#ELF-Version
> 
> I experimented with adding into include/linux/linkage.h:
> 
>   #define WEAKSECT(name) \
>     .section .text.name ASM_NL \
>     WEAK(name)
> 
>   #define ENDPROCSECT(name) \
>     ENDPROC(name) ASM_NL \
>     .previous
> 
> And then defining in arch/arm/cpu/armv7/start.S:
> 
>   WEAKSECT(save_boot_params)
>     b  save_boot_params_ret
>   ENDPROCSECT(save_boot_params)
> 
> (Note that n900 has custom non-weak save_boot_params)
> 
> Then I run:
> 
>   make CROSS_COMPILE=arm-linux-gnueabi- nokia_rx51_defconfig u-boot.bin KBUILD_LDFLAGS="--print-gc-sections"
> 
> And it showed me:
> 
>   ld: removing unused section '.text.save_boot_params' in file 'arch/arm/cpu/armv7/start.o'
> 
> So seems that it is working.
> 
> For proper integration it would be needed to integrate --sectname-subst
> flag support and then replace all usage by new macros.

That's all good to know, thanks for digging more. It would be good to
know if a quick and dirty experimental patch showed enough size savings
to be worth a more full pursuit or if we really don't have many / any
unused assembly functions or what we do have unused could be more easily
handled with an ifdef or refactoring into multiple files.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20221206/03ebabf7/attachment.sig>