cmake: Pass OPTIMIZATION_FLAG to linker too

[keith-packard]

The compiler may want to know the desired optimization level during linking, as when the compiler multilib configuration includes -Os as a selector.

[tejlmand]

Thanks for the cleanup.

It would be nice to get rid of -L<libgcc_dir>, but we should ensure there are no side-effects (can't remember in which cases this happend last time I looked at this).

A couple of code observations to be addressed in addition.

[keith-packard]

I've reworked the second patch so that the computation of rt_library and lib_include_dir are delayed until the compiler flags are finished.

I renamed library_find_path to compiler_find_path and then placed the gcc/clang implementation of that along with a new function, compiler_set_linker_properties into cmake/compiler/target_template.cmake, which gets included just after cmake/linker/target_template.cmake from FindTargetTools.cmake.

With this, the values of lib_include_dir and rt_library are both uniformly computed using --print-libgcc-file-name. As lib_include_dir is already used in the linker, that will ensure that the -L flag continues to get passed along as before, but this time it will have a value computed using the whole set of compiler flags.

This refactoring let me remove a bunch of nearly identical code in the compiler target.cmake files.

[keith-packard]

It would be nice to get rid of -L<libgcc_dir>, but we should ensure there are no side-effects (can't remember in which cases this happend last time I looked at this).

Ok, I've discovered it's actually impossible for us to compute libgcc_dir correctly in this fashion because some of the critical compiler flags are only available in the INTERFACE_COMPILE_OPTIONS property which is a generator expression, and hence cannot be evaluated during the cmake run.

Things like -m32/-m64, various -mfpu flags and many flags used on arc targets will all affect the multilib path value, but are unavailable when computing the libgcc path. I'm kinda amazed this ever worked anywhere.

For libgcc/compiler-rt, it's pretty simple - we can figure out what the runtime library basename is and use just that, without attempting to include the directory name. That should work as we can probably assume that any compiler install will use the same library name for all multilib configs?

For crtbegin.o/crtend.o, the linker won't search along the paths for them, so we really need to compute the whole path name. And that means capturing the output of the compiler front-end with the full compiler flags evaluated at build time?

[keith-packard]

Ok, I've discovered it's actually impossible for us to compute libgcc_dir correctly in this fashion because some of the critical compiler flags are only available in the INTERFACE_COMPILE_OPTIONS property which is a generator expression, and hence cannot be evaluated during the cmake run.

I did the best I could -- elided any flags which contained generator expressions and used the rest for all of the computations. That appears to at least get the flags needed for native_sim to work. A more complete solution would involve pushing the flag computation into the actual build run, and that seems 'problematic' as we'd have to pull the result back into the build process and insert it into the compile commands. Possible with make, but I'm unsure that could even work with ninja.

[keith-packard]

Btw, this is blocking an updated SDK which will use the optimization flag to select between versions of the C library, and that's part of the picolibc 1.8.10 uplevel process which will pull in RX target support, so we either need this in Zephyr or we need to decide that we're not doing any of this and to go back to the previous library selection mechanism kludge fest.

[tejlmand]

@keith-packard thanks for the effort, it's really appreciated 👍

I've been looking at the code, and in some ways we're getting a bit back to the older hassle of toolchain integration starting to depend on specific macros / functions to be available, which was addressed with the introduction of compiler properties.

However, the --print-libgcc-file-name and friends principle was not re-worked at that time, just adjusted to set the corresponding property. I will try to find a little time to take a closer look at compiler / linker property handling together with the TOOLCHAIN_C_FLAGS processing. I have some ideas.

Also I'm not too fond of the "guessing" of which compiler flags should be used when looking up the compiler dirs / which flags will impact library selection, this should be more controlled to avoid later genex surprises.

Will get back on this soon.

[keith-packard]

@keith-packard thanks for the effort, it's really appreciated 👍

Thanks.

However, the --print-libgcc-file-name and friends principle was not re-worked at that time, just adjusted to set the corresponding property. I will try to find a little time to take a closer look at compiler / linker property handling together with the TOOLCHAIN_C_FLAGS processing. I have some ideas.

There's a circular dependency here -- we need to ask the compiler to emit a path which is then used later on as a linker command line parameter. I don't think we can do that with generator expressions, which the current property mechanism uses extensively. That was the big blocker I found in this work.

Also I'm not too fond of the "guessing" of which compiler flags should be used when looking up the compiler dirs / which flags will impact library selection, this should be more controlled to avoid later genex surprises.

Agreed. Spamming the compiler with as many arguments as we can gather mostly works, but it sure doesn't seem like the right plan.

Will get back on this soon.

Can we set a deadline in the next few weeks with a plan to use this as temporary hack until you have time to fix it for real? I'm getting worried that we're going to run out of time to deliver the desired SDK features in time for the next Zephyr release.

Meanwhile, I'm adding -fsanitize=undefined support for picolibc 1.8.10 while I wait for Zephyr to be ready for that release. So at least I'm not bored?

[nashif]

Can we set a deadline in the next few weeks with a plan to use this as temporary hack until you have time to fix it for real? I'm getting worried that we're going to run out of time to deliver the desired SDK features in time for the next Zephyr release.

Meanwhile, I'm adding -fsanitize=undefined support for picolibc 1.8.10 while I wait for Zephyr to be ready for that release. So at least I'm not bored?

@tejlmand lets discuss this in toolchain WG on 14/4, @keith-packard if you can join, it would help :)

[tejlmand]

However, the --print-libgcc-file-name and friends principle was not re-worked at that time, just adjusted to set the corresponding property. I will try to find a little time to take a closer look at compiler / linker property handling together with the TOOLCHAIN_C_FLAGS processing. I have some ideas.

There's a circular dependency here -- we need to ask the compiler to emit a path which is then used later on as a linker command line parameter. I don't think we can do that with generator expressions, which the current property mechanism uses extensively. That was the big blocker I found in this work.

Yes, I see the issue.
The original intent with compiler / linker flags properties was to avoid if(CONFIG_X) checking when dealing with flag support in compiler_flags.cmake and linker_flags.cmake and instead just describe the flags, and then where they are used do the config check. The is almost true, although for historic reasons there does exists some checks.

Following this approach I think the linker should have a property describing flags which might impact path calc, but as the actual flag usage \is config dependent we need improved Kconfig -> mapping, which I guess will be on my shoulders 😃 .

For now, I made a draft to show improved linker optimization flag #89073.
It follows the existing principle from compiler flags, meaning linker_flags.cmake can define:

set_linker_property(PROPERTY optimization_speed -O2)

but also have the ability to reference the property fetching mechanism to another target, for example to re-use compiler properties, like this:

set_property(TARGET linker PROPERTY optimization_target "compiler")

I think this can replace: 894587f

and thereby at first avoid the request for implementing toolchain_linker_add_compiler_options().

Also I'm not too fond of the "guessing" of which compiler flags should be used when looking up the compiler dirs / which flags will impact library selection, this should be more controlled to avoid later genex surprises.

Agreed. Spamming the compiler with as many arguments as we can gather mostly works, but it sure doesn't seem like the right plan.

See above reply.

Can we set a deadline in the next few weeks with a plan to use this as temporary hack until you have time to fix it for real? I'm getting worried that we're going to run out of time to deliver the desired SDK features in time for the next Zephyr release.

#89073 is a first step, will take a look and the next and hopefully have something early next week.