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.

 

Chris

 

From: Michal Simek <michal.simek@amd.com>
Date: Monday, 5 February 2024 at 11:40
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.

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