- User Since
- Aug 27 2018, 3:54 PM (96 w, 3 d)
Wed, Jun 24
Fri, Jun 12
@grimar Do you know who might be a good reviewer?
Tue, Jun 9
Wed, Jun 3
We don't need to make the same change to COFF. COFF does not ignore comdats, it attempts to add a comdat symbol as long as the symbol is external. It also tracks whether a symbol is from a regular object file or bitcode file so it won't have duplicates.
Fixed a comment.
Changed variable name bitcodeComdatGroup to ltoOutputComdatGroup for consistency.
May 29 2020
@sbc100 What would be a good way to go about writing a test for this? The reason why I noticed this bug was because .debug_types are not deduplicated in thinlto, but writing a lld test case using debug metadata seems clunky.
May 28 2020
New diff: https://reviews.llvm.org/D80765
Fixed typo and removed unrelated change.
@hoy and I figured out the issue, the problem is caused by de-duplicating twice in thinlto, once in thin link, once in final link. Abandoning this in favor of a new approach. New diff coming up soon.
May 26 2020
May 20 2020
I tried building a more complex workload and the build failed due to some undefined hidden symbol. For thinlto, is the linker or codegen supposed to handle comdat folding?
Addressed comments. 1). made the same change to wasm. 2). removed the argument ignoreComdats.
May 19 2020
However the behavior before and after this change should be the same, since I believe they were being ignored before this change too
We are trying to migrate from gold to lld, and we found that when building with thinlto, lld does not de-duplicate .debug_types. De-duplication is successful with monolithic lto. Repro: clang -flto=thin -fdebug-types-section -fuse-ld=lld -Wl,-plugin-opt=-generate-type-units
Mar 11 2019
Mar 8 2019
Mar 6 2019
include/llvm/ADT/SmallVector.h:153 has an assert statement, but I'm not sure why I'm the only one who's experienced problems so far.
Jan 14 2019
Thanks for catching that!
Jan 2 2019
Could you elaborate a bit on why hot text (is that .text.hot section?) needs to have a stricter alignment requirement than the common-page-size?
Do you know why that system uses not only max-page-size but also common-page-size?
I wonder if you found a use case of -z common-page-size with a page size that is different from the ABI's default. For example, did you find a use case of the option for x86-64 with a page size larger than 4096?
Nov 30 2018
I spoke with @modocache and I'm going to take a crack at this.
Oct 26 2018
Oct 23 2018
Sep 24 2018
The changes were landed in D52433
@courbet , @lebedev.ri , I'm trying to commit the patch by first running "arc patch D51550", but it keeps saying unable to apply patch because llvm/trunk/test/Transforms/MergeICmps/X86/alias-merge-blocks.ll does not exists in index. How should I go about this?
Going back to previous revision to include the test in this revision.
Sep 20 2018
Thanks for the fix!
Sep 18 2018
Sep 13 2018
@rsmith , thanks for the review, I fixed the variable capitalization. If you could land it for me that'll be awesome!
Fixed diff dependencies.
Addressed @lebedev.ri 's comment and added dependent child diff to show original check-lines.
Addressed @rsmith 's comments.
Sep 12 2018
Emit non-fatal error for typo if file exists.
And of course thank you to @trentxintong :)
Thanks for the review! @efriedma
Addressed @courbet comment
Sep 10 2018
Removed clang-format changes unrelated to this patch.
For instructions that may have side effects, check that the instruction is a simple load or simple store before checking for alias. For things like function calls, we would have to check that they don't throw and have no read/write to memory. The function calls are potentially nested as well, it's probably better to just bail on those cases.
Sep 7 2018
Friendly ping @courbet :)
Aug 31 2018
Aug 29 2018
Merged warning with existing file_not_found_error.