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 <tr...@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 <p...@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
signature.asc
Description: PGP signature