- User Since
- Jul 25 2016, 12:54 PM (155 w, 6 d)
Fri, Jul 19
Tue, Jul 16
LGTM, although you still have to rebase it past the change to camelBack casing of variables :-)
Mon, Jul 15
Thanks! Seems to build fine for me now.
Sun, Jul 14
This triggers failed asserts in some cases, reproable with https://martin.st/temp/loadimage-preproc.cpp.xz, with clang++ -target i686-w64-mingw32 -c -O3 loadimage-preproc.cpp. Will post a proper bug report later today, unless someone else beats me to it.
Fri, Jul 12
Thanks for the quick action, this is almost perfect! In my env I had to add an explicit stddef.h include (which didn't cause trouble with use of free) in order to get a typedef of size_t.
Sorry for not testing out this commit until it hit the tree and not commenting on this before. This one turned out to break MinGW builds for a few different reasons, that I'll comment on inline.
Thu, Jul 11
Mon, Jul 8
Any other concerns with the patch, or is it ok as is?
Sun, Jul 7
Sat, Jul 6
Mon, Jul 1
Fri, Jun 28
Thu, Jun 27
Added the comment Rui requested. I also renamed the obj file from .obj to .o, as it is created with windres, not cvtres.
Wed, Jun 26
Tue, Jun 25
Mon, Jun 24
I'd suggest skipping the lld condition, the version script adds no value here.
Actually, it turns out that GNU windres does things differently - GNU ld can link two resource object files from GNU windres, and resources from both end up reachable. When faced with two resource files from llvm-cvtres, only resources from the first one ends up accessible. In the converse case, if lld is given a resource object file from GNU windres (even a single one), those resources aren't reachable at all.
Sun, Jun 23
Actually, I'm leaning towards making this only a warning in MinGW cases. If multiple resource objects are passed and the .rsrc$01 sections are concatenated, is the effect that only the first one actually is read and used, or what does happen?
Sat, Jun 22
link.exe's /wholearchive semantics on the other hand mean that it wouldn't load the resource object files from the .lib file at all. Since this scenario is probably still an unlikely corner case, the difference in behavior here seems fine -- and lld-link might have to change to use link.exe's /wholearchive semantics in the future anyways.
Jun 21 2019
Jun 20 2019
I just ran into a performance regression due to this commit. With https://martin.st/temp/glew-preproc-i686.c, compiling with clang -target i686-w64-mingw32 -c -O3 glew-preproc-i686.c went from 73 seconds before this commit, to 133 seconds afterwards.
Jun 16 2019
FWIW, I ran into one error with this on VLC. In one case, a static library ends up (erroneously) containing a resource object file. This static library is included with the --whole-archive option (indirectly, via lots of libtool magic) when linking a DLL, which also contains a resource object files of its own.
Jun 14 2019
This caused a regression, where compilation now crashes. See https://bugs.llvm.org/show_bug.cgi?id=42279 for details.
Jun 13 2019
Jun 11 2019
The only real way to run into this is if users manually convert .res files to .obj files by running cvtres and then handing the resulting .obj files to lld-link instead, which in practice likely never happens.
Jun 10 2019
Avoid expanding unless no -m option was found.
Expand only in isPETarget, expand in the mingw driver as well.
Localized the extra expansion to the ELF case, use the host platform native quoting style.
Jun 8 2019
Jun 7 2019
Adjusted to make it work more like how this is implemented in the ELF linker; still failing (and properly so) if there's real undefined references to a symbol that was requested with the new option (-u in GNU ld).
I don't know this area so I can't comment on it from that perspective, but it does indeed speed up my case, to even faster than it was before the regression. On my system, it originally took 75 s to compile, 220 s after the regression, and now 62 s with this patch. So looking good in that aspect at least!
Jun 6 2019
Looks good and thought through. I had a few nits though.
Mimic the real -u option behavior properly.
Jun 5 2019
This caused a serious compile time regression. https://martin.st/temp/glew-preproc.c used to take 75 s to compile, now it takes 220 s. Repro with clang -target x86_64-w64-mingw32 -c -O3 glew-preproc.c.
Jun 4 2019
Can you add a testcase for this? Also it's appreciated if the uploaded diffs are made with a larger context (diff -U999 or similar) to allow expanding the context here in phabricator.
Jun 1 2019
This change had another breaking effect as well, for the MinGW target. Code that implements a COM interface easily ends up overriding a __declspec(nothrow) function with a method that in most cases (at least in cases that follow the microsoft sample code) lacks the same attribute. For code with -fms-extensions, this is a warning (as all exception spec mismatches are non-fatal there), but MinGW code doesn't necessarily use this option. See https://bugs.llvm.org/show_bug.cgi?id=42100 for full issue report and discussion.
May 31 2019
This change broke compiling Qt on MinGW, see https://bugs.llvm.org/show_bug.cgi?id=42089. Trivially reproducible by trying to compile a snippet that looks like this:
May 30 2019
Please add testcases for these changes. You could e.g. change some of the existing tests in test/tools/llvm-rc so that both forms are exercised, or add a new test there that uses that form.
May 29 2019
Testcase and slightly longer description is available at https://bugs.llvm.org/show_bug.cgi?id=42065.