On Mon, May 24, 2021 at 09:19:35PM +0200, Marek Behun wrote: > On Mon, 24 May 2021 13:44:38 -0400 > Tom Rini wrote: > > > On Mon, May 24, 2021 at 01:09:19PM -0400, Tom Rini wrote: > > > On Mon, May 24, 2021 at 05:58:55PM +0200, Marek Behun wrote: > > > > On Mon, 24 May 2021 11:40:53 -0400 > > > > Tom Rini wrote: > > > > > > > > > On Fri, May 21, 2021 at 12:56:41PM -0400, Tom Rini wrote: > > > > > > On Fri, May 21, 2021 at 06:00:31PM +0200, Marek Behún wrote: > > > > > > > On Fri, 21 May 2021 10:11:47 -0400 > > > > > > > Tom Rini wrote: > > > > > > > > > > > > > > > On Thu, May 20, 2021 at 01:56:29PM -0500, Adam Ford wrote: > > > > > > > > > On Thu, May 20, 2021 at 6:25 AM Marek Behún > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Enable LTO for some boards that were tested by people on U-Boot > > > > > > > > > > Mailing List. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Marek Behún > > > > > > > > > > Tested-by: Adam Ford > > > > > > > > > > Tested-by: Pali Rohár > > > > > > > > > > Tested-by: Tim Harvey > > > > > > > > > > > > > > > > > > Since the imx8mm beacon boards and the imx8mm venice board both show > > > > > > > > > promise, does it make sense to 'imply' the LTO for anything enabling > > > > > > > > > imx8mm? > > > > > > > > > Same thing for the various omap3 boards, and potentially the renesas > > > > > > > > > RZ/G2 boards. I know Tom went through to remove a bunch of boards > > > > > > > > > that were never converted to DM. Most of the boards remaining > > > > > > > > > boards have minimal board files and most of code is common to other > > > > > > > > > boards in the same platforms. > > > > > > > > > > > > > > > > > > I have an l138_lcdk that I can use to test which I expect to be > > > > > > > > > similar to the da850evm. > > > > > > > > > > > > > > > > As much as I am eager to move everything, quickly, over to LTO by > > > > > > > > default, I think the problems that we've seen thus far show it's best > > > > > > > > to really make it an explicit enable per board at least for the first > > > > > > > > release or two. Once we've hopefully gotten more boards tested and > > > > > > > > enabled we can see what makes sense for defaults, give a release worth > > > > > > > > of heads up, and then go. > > > > > > > > > > > > > > Tom, are there some other issues aside from the one failing CI scenario > > > > > > > (sandbox_clang)? Would you be willing to merge this if I resolved that > > > > > > > one fail by disabling LTO for that scenario (until I resolve it)? It > > > > > > > would help me not having to maintain all 30+ patches... > > > > > > > > > > > > Yeah, CI needs to keep passing, so if we need to disable > > > > > > sandbox+clang+lto for now, OK. > > > > > > > > > > Ah, I see the problem now. I've worked out a fix after looking at the > > > > > Linux kernel a bit and I'll post something for us and upstream dtc as > > > > > well. > > > > > > > > > > > > > What do you mean? The problem is in dtc? I see 2 problems: > > > > - one with DM test > > > > - one with stack protector test > > > > > > I don't have a full answer about the stack protector test just yet, but > > > it almost seems like it's too simple and maybe something is happening > > > with it being optimized to not a problem? > > > > Yeah, so clang with LTO optimizes away that memset call, and so the test > > passes. I'll do something to make sure the array is used so it won't be > > optimized away. > > > > I am unable to make the compiler to protect the stack of that function > even with GCC on my local machine. It seems that at least on my gentoo > with gcc-10.2, when compiling with -ffreestanding, the call to > __stack_chk_fail is not made at all. > > I even started reading sources of gcc on thursday because of this, but > it didn't lead anywhere... > > When you compile sandbox_defconfig with gcc, does the test pass on your > local machine? It passes here with gcc-7.5 as well as gcc-9.2.0 in CI. When I moved to a gcc-11 snapshot in CI, it was also passing there before, so I'm not sure what's going on in your setup. But as part of coming up with the patch I sent to fix the test, I ran it manually and saw the expected failure message. -- Tom