Hi Chris,

On 2/2/24 12:17, Chris Kay wrote:

Hi Prasad,

ENABLE_LTO places constraints on the toolchain (both the linker and the compiler must be aligned on the format of the generated object files), so it cannot therefore be configured from the platform makefile, which loads after the toolchain has already been set up.

I get it but on the other hand when we are running out of space. Over years simply TF-A is growing and enabling LTO give us time and space to deprecate, remove some parts to stay inside limits. I am talking about one specific configuration when TF-A runs out of OCM. I don't think toolchain dependency is really a big deal for me. If toolchain doesn't support LTO what can go wrong? I expect size is going to be bigger and it will fail to compile. Or toolchain just failed with unknown flag. For both we can simply simply document this limitation.

If LTO is supported for GCC version X.Y then it should be just fine to describe this dependency and make it mandatory for this configuration only.

What do you think?

Thanks, Michal

Hi Chris,

On 2/5/24 12:32, Chris Kay wrote:

Hi Michal,

It’s less about the toolchain dependency, and more about the fact that we currently do toolchain discovery, then toolchain configuration (of which setting the base CFLAGS is a part, including -flto), and _/then/_ the platform file gets loaded in, which means that if ENABLE_LTO has not been configured before the platform file has been loaded then it is already too late.

I did try enabling LTO by default last year (https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/20540 https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/20540) but it triggered a number of test failures and I ran out of time to investigate further. Perhaps we could revive that?

Of course I am fine with this but it is obviously much harder/complicated task to enable something by default compare to enabling it for one specific configuration. Also much easier to validate.

What would be the next step with your patch? Just rebase it and run CI over it again?

Thanks, Michal

On 2/5/24 13:04, Chris Kay wrote:

Assuming it still fails to pass then we could perhaps move the LTO check somewhere after the platform file is included and hope for the best. It’s a bit spaghetti and it would break any platforms that read ENABLE_LTO, but I don’t think there are any so cross fingers and hope for the best.

With that said, we also ought to address any failures anyway because they are generally indicative of UB or broken linker scripts.

I am reading it that you will do this step and keep us in CC and we are happy to test and help and we will definitely set it up for that one configuration.

Thanks, Michal

Sorry for the delay, folks – we did some pipecleaning of some changes that conflicted with this. I’ve pushed the following change, which should enable this:

https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/26693

Chris

From: Chris Kay via TF-A tf-a@lists.trustedfirmware.org Date: Monday, 5 February 2024 at 14:27 To: Michal Simek michal.simek@amd.com, Kummari, Prasad Prasad.Kummari@amd.com, tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org Cc: Nagal, Amit amit.nagal@amd.com, Belsare, Akshay akshay.belsare@amd.com Subject: [TF-A] Re: ENABLE_LTO feature usage. I’ve triggered CI+2 on the LTO-by-default patch, but it’s encountering build failures already: https://ci.trustedfirmware.org/job/tf-a-gerrit-tforg-l2/1689/

I’ll try to find some time to move the LTO configuration after platform file inclusion later this week, unless anybody else is willing to volunteer to get it sorted faster.

Chris

From: Michal Simek michal.simek@amd.com Date: Monday, 5 February 2024 at 12:21 To: Chris Kay Chris.Kay@arm.com, Kummari, Prasad Prasad.Kummari@amd.com, tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org Cc: Nagal, Amit amit.nagal@amd.com, Belsare, Akshay akshay.belsare@amd.com Subject: Re: ENABLE_LTO feature usage.

On 2/5/24 13:04, Chris Kay wrote:

Assuming it still fails to pass then we could perhaps move the LTO check somewhere after the platform file is included and hope for the best. It’s a bit spaghetti and it would break any platforms that read ENABLE_LTO, but I don’t think there are any so cross fingers and hope for the best.

With that said, we also ought to address any failures anyway because they are generally indicative of UB or broken linker scripts.

I am reading it that you will do this step and keep us in CC and we are happy to test and help and we will definitely set it up for that one configuration.

Thanks, Michal

Hi Chris,

With latest upstream master with below patch you have suggested we got some link time errors as mentioned in logs below. However with another patch https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/26703, this link errors got resolved. We can now enable ENABLE_LTO flag in platform.mk. Once all related patches are merged upstream, we will submit our platform specific patch for ENABLE_LTO.

Logs: (.text.asm.handle_interrupt_exception+0x20): undefined reference to `plat_ic_get_pending_interrupt_type' (.text.asm.handle_interrupt_exception+0x20): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `plat_ic_get_pending_interrupt_type' (.text.asm.handle_interrupt_exception+0x2c): undefined reference to `get_interrupt_type_handler' (.text.asm.handle_interrupt_exception+0x2c): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `get_interrupt_type_handler' /aarch64-xilinx-elf-ld.bfd.real: trusted-firmware-a/build/zynqmp/release/bl31/crash_console_helpers.o: in function `plat_crash_console_putc': (.text.asm.plat_crash_console_putc+0x18): undefined reference to `console_list'

Regards, Prasad.

From: Chris Kay Chris.Kay@arm.com Sent: Monday, February 26, 2024 8:50 PM To: Simek, Michal michal.simek@amd.com; Kummari, Prasad Prasad.Kummari@amd.com; tf-a@lists.trustedfirmware.org Cc: Nagal, Amit amit.nagal@amd.com; Belsare, Akshay akshay.belsare@amd.com Subject: Re: ENABLE_LTO feature usage.

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Sorry for the delay, folks - we did some pipecleaning of some changes that conflicted with this. I've pushed the following change, which should enable this:

https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/26693

Chris

From: Chris Kay via TF-A mailto:tf-a@lists.trustedfirmware.org Date: Monday, 5 February 2024 at 14:27 To: Michal Simek mailto:michal.simek@amd.com, Kummari, Prasad mailto:Prasad.Kummari@amd.com, mailto:tf-a@lists.trustedfirmware.org mailto:tf-a@lists.trustedfirmware.org Cc: Nagal, Amit mailto:amit.nagal@amd.com, Belsare, Akshay mailto:akshay.belsare@amd.com Subject: [TF-A] Re: ENABLE_LTO feature usage. I've triggered CI+2 on the LTO-by-default patch, but it's encountering build failures already: https://ci.trustedfirmware.org/job/tf-a-gerrit-tforg-l2/1689/   I'll try to find some time to move the LTO configuration after platform file inclusion later this week, unless anybody else is willing to volunteer to get it sorted faster.   Chris   From: Michal Simek mailto:michal.simek@amd.com Date: Monday, 5 February 2024 at 12:21 To: Chris Kay mailto:Chris.Kay@arm.com, Kummari, Prasad mailto:Prasad.Kummari@amd.com, mailto:tf-a@lists.trustedfirmware.org mailto:tf-a@lists.trustedfirmware.org Cc: Nagal, Amit mailto:amit.nagal@amd.com, Belsare, Akshay mailto:akshay.belsare@amd.com Subject: Re: ENABLE_LTO feature usage.

On 2/5/24 13:04, Chris Kay wrote:

Assuming it still fails to pass then we could perhaps move the LTO check somewhere after the platform file is included and hope for the best. It's a bit spaghetti and it would break any platforms that read ENABLE_LTO, but I don't think there are any so cross fingers and hope for the best.

With that said, we also ought to address any failures anyway because they are generally indicative of UB or broken linker scripts.

I am reading it that you will do this step and keep us in CC and we are happy to test and help and we will definitely set it up for that one configuration.

Thanks, Michal