[MinGW] Always enable -mbig-obj for LLVM build unless using Clang

[mati865]

Summary

It's easy to hit 2**16 limit with i686 GNU toolchains these days.
Clang does it automagically so we skip this flag to avoid unused flag warnings.

[mstorsjo]

Would you care to describe a bit more about the different contexts involved here:

• Previously this was only enabled for 64 bit cases (apparently because with SEH, there are a bit more individual sections than in non-SEH cases?). Does this stem from the fact that we enabled -ffunction-sections, which crosses the limit on i686 too?
• We don't enable it on clang at all (but we did before). IIRC the option is a no-op on clang, and big objects are created where needed only, right?

I'd like to have both of these bits explained in the commit message - can you try to formulate something? Technically I think the change is ok (pending answers to above).

[mati865]

Sorry I thought it's simple enough on it's own.

Previously this was only enabled for 64 bit cases (apparently because with SEH, there are a bit more individual sections than in non-SEH cases?). Does this stem from the fact that we enabled -ffunction-sections, which crosses the limit on i686 too?

I just thought why not upstream the patch we've been carrying since 2018 at MSYS2. We have been having recurring error about sections limit, possibly related to some specific LLVM options.
TBH I haven't tested it with main and we haven't enabled -ffunction-sections.

We don't enable it on clang at all (but we did before). IIRC the option is a no-op on clang, and big objects are created where needed only, right?

Exactly, Clang was actually the main motivator to do something about it since there are over 100 useless warnings in our build saying: clang: warning: argument unused during compilation: '-Wa,-mbig-obj' [-Wunused-command-line-argument].

I can revert pointer size check because reproducing and reducing this issue is not worth the time it will take - we can carry it in the repo until we drop 32-bit support.
Avoiding ~30% of the warnings in our build is more important thing for me.

[mstorsjo]

In D102419#2758398, @mati865 wrote:

Sorry I thought it's simple enough on it's own.

Previously this was only enabled for 64 bit cases (apparently because with SEH, there are a bit more individual sections than in non-SEH cases?). Does this stem from the fact that we enabled -ffunction-sections, which crosses the limit on i686 too?

I just thought why not upstream the patch we've been carrying since 2018 at MSYS2. We have been having recurring error about sections limit, possibly related to some specific LLVM options.

Oh, ok, that’s certainly possible that we’re close to/past the limit even in i686 builds these days, even before that new option

TBH I haven't tested it with main and we haven't enabled -ffunction-sections.

We don't enable it on clang at all (but we did before). IIRC the option is a no-op on clang, and big objects are created where needed only, right?

Exactly, Clang was actually the main motivator to do something about it since there are over 100 useless warnings in our build saying: clang: warning: argument unused during compilation: '-Wa,-mbig-obj' [-Wunused-command-line-argument].

I can revert pointer size check because reproducing and reducing this issue is not worth the time it will take - we can carry it in the repo until we drop 32-bit support.
Avoiding ~30% of the warnings in our build is more important thing for me.

Nah, I’m fine with changing it for 32 bit, it’s ok as a collateral change when touching this. But warnings in builds with clang was the reason I was looking for here. I’m not getting such warnings, that’s why I didn’t realize that this was the motivator.

My builds are done with “-Qunused-arguments", https://github.com/mstorsjo/llvm-mingw/blob/master/wrappers/clang-target-wrapper.sh#L89, because of how my wrapper sets defaults. But I can’t reproduce that warning even when bypassing my wrapper, e.g. “clang -target x86_64-w64-mingw32 -c -Wall -Wextra -Weverything -Wa,-mbig-obj test.c” doesn’t give such warnings.

[mati865]

I don't know when they exactly appear but look at my building log: https://reviews.llvm.org/P8263
The build was done with 16 threads so it's hard to tell which objects made it appear.

[mstorsjo]

In D102419#2759985, @mati865 wrote:

I don't know when they exactly appear but look at my building log: https://reviews.llvm.org/P8263
The build was done with 16 threads so it's hard to tell which objects made it appear.

When building with ninja, the output is buffered so the warnings appear directly after the specific build target that cause them. I was able to reproduce it with my toolchains also, when removing the -Qunused-arguments that I have in the clang wrapper - the warning is printed when linking.

The change LGTM, I'll amend the commit message before pushing, to something like this:

Clang does it automagically, so it's not needed there, and the option causes warnings about being unused when linking.

[mati865]

When building with ninja, the output is buffered so the warnings appear directly after the specific build target that cause them.

TIL, that's really nice BTW.

Thank you for committing.

[mati865]

llvm/cmake/modules/HandleLLVMOptions.cmake

415

I did not regenerate CMake files properly between runs.
This line should actually read: if (NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang").

I'm sorry for that.

[mstorsjo]

llvm/cmake/modules/HandleLLVMOptions.cmake

415

Ok, I'll push a correction a bit later.

llvm/cmake/modules/HandleLLVMOptions.cmake

if(MSVC)
   # CMake previously automatically set this value for MSVC builds, but the
   # behavior was changed in CMake 2.8.11 (Issue 12437) to use the MSVC default
   # value (1 MB) which is not enough for us in tasks such as parsing recursive
   # C++ templates in Clang.
   set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /STACK:10000000")
 elseif(MINGW) # FIXME: Also cygwin?
   set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--stack,16777216")
 
-  # Pass -mbig-obj to mingw gas on Win64. COFF has a 2**16 section limit, and
-  # on Win64, every COMDAT function creates at least 3 sections: .text, .pdata,
-  # and .xdata.
-  if (CMAKE_SIZEOF_VOID_P EQUAL 8)
+  # Pass -mbig-obj to mingw gas to avoid COFF 2**16 section limit.
+  if (NOT CLANG)
     append("-Wa,-mbig-obj" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
   endif()
 endif()
 
 option(LLVM_ENABLE_WARNINGS "Enable compiler warnings." ON)
 
 if( MSVC )
   include(ChooseMSVCCRT)