On Fri, 15 Nov 2024 10:12:07 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> I misspoke a bit earlier, I do not think LTO will become the Standard way of >> compiling HotSpot, even if all the issues with it are fixed. It simply takes >> too long, which I think is the main reason Erik mentioned why it is not used >> for the product today. I intend to make LTO viable, for those who want the >> extra performance at the expense of link times, but it is unlikely to become >> the default >> >> The documentation linked above is for incremental linking, it is indeed >> possible to link LTO and non LTO object files together if linking normally >> and not incrementally, otherwise LTO would already fail horribly on assembly >> files (Which are always assembled directly into non LTO object files), and >> HotSpot contains many assembly source files (This might be an issue for >> Magnus and his static Java, though, since I think that relies on incremental >> linking). However, that said, I am not fond of disabling LTO for any >> specific file, since that will cause a decline in the resulting assembly >> quality >> >> I've been looking at alternative ways to implement poisoning of methods we >> don't want people to be using that don't rely on the warning attribute, >> however that hasn't been successful >> ([8313396](https://bugs.openjdk.org/browse/JDK-8313396)). I've checked the >> gcc bug tracker, and the current warnings are apparently a hard to fix bug >> in gcc, since pragmas are not saved into the GIMPLE when LTO is enabled. >> I'll list the poisoning thing as a blocker for making LTO viable, so the >> issues with LTO won't be fixed until the poisoning works with LTO. I'll look >> into poisoning again once my Windows fixes make it in >> >> As a workaround that doesn't rely on disabling poisoning whatsoever, you >> could NOINLINE code that contains an ALLOW_C_FUNCTION, but that would >> likewise also inhibit optimizations for the JVM > > LTO seems to be less than a factor of 2 longer build times. If it makes a > measurable difference in performance, that seems like a clear win for > production builds. Certainly hotspot developers wouldn't want it on, but we > will know about that, just like using ccache and not using precompiled headers > for incremental development. > > Thanks for clarifying the incremental linking thing. > > The relevant gcc bug regarding pragmas not carrying through to LTO is > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922 > #pragma diagnostic ignored not honoured with -flto > It's been around for a few years now, with no obvious progress. > > I failed to understand the proposal for 8313396. I took another look today, > and I'm still not getting it. Maybe we should talk about it over there. For the record, I'm seeing 3 warnings, 2 of which are for the same code. Both are -Walloc-size-larger-than= warnings for calls to operator new[]. In gtestMain.cpp, in init_jvm, `new JavaVMOption[num_jvm_options]` is being warned about. The warning is obviously wrong, as the value it is complaining about uint64_max, while the variable has int type. The other is in (non-Windows) CreateArgvFromArgs in gtest-death-test.cc, and is being too conservative about value bounds, so also wrong. So both of these appear to be LTO bugs. Of course, it's not possible to suppress these warnings via pragmas, because of the bug that LTO doesn't honor such. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22069#discussion_r1843567653
