upload correct diff

Great, thanks!

The reason it's important that zerofill sections are at the end is because dyld does

clang-format

FWIW https://lab.llvm.org/buildbot/#/builders/123 has been red for several days after this landed too (eg https://lab.llvm.org/buildbot/#/builders/123/builds/4545)

In D104353#2828411, @thakis wrote:

Looks like this breaks tests on some windows bots: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8844046562894240432/+/u/package_clang/stdout?format=raw

$ ":" "RUN: at line 19"
$ "ld64.lld" "-arch" "x86_64" "-platform_version" "macos" "10.15" "11.0" "-syslibroot" "C:/b/s/w/ir/cache/builder/src/third_party/llvm/lld/test\MachO\Inputs\MacOSX.sdk" "-fatal_warnings" "-lSystem" "C:\b\s\w\ir\cache\builder\src\third_party\llvm-bootstrap\tools\lld\test\MachO\Output\force-load.s.tmp/foo.o" "-force_load" "C:\b\s\w\ir\cache\builder\src\third_party\llvm-bootstrap\tools\lld\test\MachO\Output\force-load.s.tmp/foo.a" "-force_load" "C:\b\s\w\ir\cache\builder\src\third_party\llvm-bootstrap\tools\lld\test\MachO\Output\force-load.s.tmp/foo.a" "C:\b\s\w\ir\cache\builder\src\third_party\llvm-bootstrap\tools\lld\test\MachO\Output\force-load.s.tmp/test.o" "-o" "/dev/null"
# command stderr:
ld64.lld: error: duplicate symbol: _bar

>>> defined in C:\b\s\w\ir\cache\builder\src\third_party\llvm-bootstrap\tools\lld\test\MachO\Output\force-load.s.tmp/foo.a(archive-foo.o)

>>> defined in C:\b\s\w\ir\cache\builder\src\third_party\llvm-bootstrap\tools\lld\test\MachO\Output\force-load.s.tmp/foo.a(archive-foo.o)

Looks like this breaks tests on some windows bots: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8844046562894240432/+/u/package_clang/stdout?format=raw

We also see check-all timeout recently (fairly consistently), see https://bugs.chromium.org/p/chromium/issues/detail?id=1221702

FYI, strip needs this too. Without this change, the data-in-code change regressed stripping again, with this error:

% strip out/gn/bin/lld
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/strip: fatal error: file not in an order that can be processed (data in code info out of place): /Users/thakis/src/llvm-project/out/gn/bin/lld

Thanks!

Looks like this breaks tests on mac: http://45.33.8.238/macm1/11921/step_11.txt

Looks like this breaks tests on windows: http://45.33.8.238/win/40255/step_11.txt

That's a good argument. lgtm.

is there any documentation on how to use this? I see that clang has a -forder-file-instrumentation switch to enable this. But what do I do after I enabled it? Run the instrumented program and then…? From the name, I'm guessing maybe the idea is that this will produce an order file?

is there any documentation on how to use this? I see that clang has a -forder-file-instrumentation switch to enable this. But what do I do after I enabled it? Run the instrumented program and then…? From the name, I'm guessing maybe the idea is that this will produce an order file?

Thanks!

Out of interest, how do you end up with an obj file with several of these?

(I agree with MaskRay that the ELF behavior is nicer, but you solve the problems you have, not the ones you wish you had.)

Nice, that's much less convoluted than I had feared :)

Do you need someone to commit this for you, or do you have commit permissions.

Seems fine, but can you change the patch description to say "When writing a map file, sort symbols in parallel" to make clear that this only affects map file writing?

Looks like this lacks test coverage if it needs no test update to go along with it?

Either this or your concurrent commit broke check-clang: http://45.33.8.238/linux/48839/step_7.txt

I'm a bit surprised that this has such a big effect.

In D103977#2814584, @int3 wrote:

But overall, it feels like things get a lot more complicated because we're not creating real InputSections for each literal in literal sections.

I agree, it's quite unfortunate...

Are there so many more literals than normal symboled inputsections? What's the memory / perf hit from just having normal InputSections for each literal?

Good questions! For chromium_framework, here are the relative counts of literals and subsections before deduplication (generated via D104158):

word literals: 145224 (8%)
cstring literals: 353031 (20%)
subsections: 1260720 (72%)

So actual subsections are still by far the largest, but cstrings take up a sizable chunk regardless. I also hacked up a diff that creates one InputSection per string, and even without doing dedup, it's already slower: D104159

The remaining question is... could we trim InputSection's size and close this performance gap? There are definitely opportunities here (e.g. we could replace name, segname, and flags with a pointer to the original section_64 header.) But there's still quite a number of other fields that I doubt can be removed. So while this is not a watertight analysis, I think it's enough to make a case for landing this architectural change.

You can look at http://45.33.8.238/ , it's public and the Linux bit on it cycles in a little over 3 minutes :)

One of your 4 commits broke the build: http://45.33.8.238/linux/48738/step_4.txt

This seems to break tests everywhere, eg http://45.33.8.238/linux/48721/step_12.txt

In D104084#2813716, @gkm wrote:

In D104084#2813364, @thakis wrote:

The intelligent approach isn't _that_ hard, is it? It's < 200 lines in ld64 I think. ld64-linked Chromium Framework leaves some __eh_frame around, so I think removing the whole section generally probably doesn't work.

Yes, I will do it, but wanted this out quickly. (I changed the description to read "... unless and until (maybe later today!) we have a use case ..." :)

When I looked at the CU entries for Chromium Framework, I saw none that had type bits [24..27] == 4, which means UNWIND_X86_64_MODE_DWARF. What am I not seeing?

In D103184#2804241, @jcai19 wrote:

In D103184#2803768, @thakis wrote:

FWIW the failure goes away locally if I revert this change here, so it's definitely due to this change here.

Things have been red for a while, probably good to revert while you investigate by this point.

Sorry for the delay but my macbook had some issues so I just finished building LLVM, but I can't seem to reproduce the test failure locally (neither by running the test directly with llvm-lit nor with ninja check-clang). In fact I don't quite understand why this test failed. Like @nickdesaulniers mentioned the build failure should have been fixed in https://reviews.llvm.org/rG76d9bc72784d88f4dd57b9939e52c73739438af5 for a different patch. Do you mind sharing the instructions you used to reproduce the test failure? Thanks.

The intelligent approach isn't _that_ hard, is it? It's < 200 lines in ld64 I think. ld64-linked Chromium Framework leaves some __eh_frame around, so I think removing the whole section generally probably doesn't work.

LG with tweak below

I tried patching this in

(Also FYI if you use the "Edit Related Revisions…" button in the upper right on phab, the presubmit bots can correctly handle dependent changes)

This change as-is is fine.

Nice!