Page MenuHomePhabricator

bd1976llvm (ben)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 18 2017, 9:47 AM (167 w, 6 d)

Recent Activity

Today

bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Use IR rather than C for the clang test.
I was able to simplify the clang patch somewhat as a "lowering error" isn't any more revealing than just emitting a generic "backend error".

Tue, Apr 7, 4:22 PM · Restricted Project
bd1976llvm added a comment to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

I am mostly fine with the LLVM CodeGen/MC side change, but the clang side change can probably be moved to a subsequent change. For the test/clang/CodeGen/ test, we may want to test .ll -> .s instead, not .c -> .s

Tue, Apr 7, 4:22 PM · Restricted Project

Yesterday

bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Report error using LLVM diagnostics framework.

Mon, Apr 6, 3:49 PM · Restricted Project

Thu, Apr 2

bd1976llvm added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Thu, Apr 2, 6:27 PM · Restricted Project

Wed, Apr 1

bd1976llvm added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Wed, Apr 1, 6:32 PM · Restricted Project

Tue, Mar 31

bd1976llvm added a comment to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

@nickdesaulniers and @MaskRay,

Tue, Mar 31, 12:33 PM · Restricted Project
bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

use reportError instead of report_fatal_error.
Split testing of error and output to defeat redirection race.

Tue, Mar 31, 12:33 PM · Restricted Project
bd1976llvm added a comment to rG8cedf0e2994c: Reland "[Support] make report_fatal_error `abort` instead of `exit`".

Thanks for the comments, @arsenm. I expected this would cause concerns for practical reasons. I do think it is heading the right direction though.

I have a problem with this change. IMO the entire reason to use report_fatal_error is to not crash, or do anything that looks like a crash.

6c2d233e7a8993d90fd97703ed8331f2d6e80671 introduced this method. You could tell from the deleted code that it was trying to replace abort() call. MHO is that report_fatal_error should crash (or could since crash is its default behavior, it could be overriden) as its name suggests. It is for shortcutting error propagation where it is hard to assume the compilation process could exit cleanly. So exit may not be a good choice.

I should never see a backtrace after calling report_fatal_error, and it should be a clean exit.

This part is customized by tools using install_fatal_error_handler (clang does). llc uses InitLLVM which asks for a backtrace.

For example I should not see a backtrace on a legalization failure with -global-isel-abort=1

I think you mean in this case, the crash better not to trigger a backtrace? The option name suggests it should abort.

I think there are 3 levels of error handling:

  1. User facing errors that can occur due to user error, that should use the LLVMContext error reporting
  2. Clean errors that generally should generally not be end user facing, but also should be handled gracefully and in all builds. Most cases that can only be produced by writing unrealistic MIR tests should use this form of error since it's not really worth the effort to produce a user facing error
  3. Asserts for situations that should never happen. This is what llvm_unreachable/assert, and can be deleted in release builds.

    This patch blobs uses 2 and 3 together. report_fatal_error is not equivalent to an assert.
Tue, Mar 31, 9:22 AM

Mon, Mar 30

bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Addressed header file order review comment

Mon, Mar 30, 7:08 PM · Restricted Project
bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Addressed review comments.
Cosmetic test improvements.

Mon, Mar 30, 7:08 PM · Restricted Project
bd1976llvm added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Mon, Mar 30, 7:08 PM · Restricted Project

Thu, Mar 26

bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Avoided use of ,unique, assembly feature when not using the integrated assembler.

Thu, Mar 26, 7:35 PM · Restricted Project

Wed, Mar 25

bd1976llvm added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Wed, Mar 25, 6:25 PM · Restricted Project
bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Corrected silly mistake.

Wed, Mar 25, 5:53 PM · Restricted Project

Mon, Mar 23

bd1976llvm added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Mon, Mar 23, 11:28 AM · Restricted Project
bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Added llvm_unreachable to getSectionPrefixForGlobal()

Mon, Mar 23, 11:28 AM · Restricted Project

Fri, Mar 20

bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Rebased and removed changes to MCContext::renameELFSection().

Fri, Mar 20, 4:18 AM · Restricted Project

Thu, Mar 19

bd1976llvm added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Thu, Mar 19, 1:09 PM · Restricted Project
bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Thu, Mar 19, 12:32 PM · Restricted Project

Tue, Mar 17

bd1976llvm updated the summary of D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Tue, Mar 17, 8:31 AM · Restricted Project
bd1976llvm updated the summary of D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Tue, Mar 17, 8:31 AM · Restricted Project
bd1976llvm updated the summary of D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Tue, Mar 17, 8:31 AM · Restricted Project

Mon, Mar 16

bd1976llvm added a comment to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Addressed review comments and rebased onto master.

Mon, Mar 16, 9:14 AM · Restricted Project
bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Mon, Mar 16, 9:14 AM · Restricted Project

Tue, Mar 10

bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Tue, Mar 10, 12:33 PM · Restricted Project
bd1976llvm added a comment to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

I saw this a couple of weeks ago. Apologies that I did not pay enough attention. (I complained that GNU as lacks features. Things are changing (I'm glad that my __patchable_function_entries complaints worked:) )

Tue, Mar 10, 12:00 PM · Restricted Project
bd1976llvm updated the summary of D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Tue, Mar 10, 9:12 AM · Restricted Project

Feb 10 2020

bd1976llvm added a comment to D73999: [MC][ELF] Error for sh_type, sh_flags or sh_entsize change.

Address review comments.

I tend to agree with Alan Modra's arguement in
https://sourceware.org/ml/binutils/2020-02/msg00093.html

I don't think so. User assembly often gets section attributes wrong
or leaves them off entirely for special sections known to the
assembler. ie. the first .section .foo above is a built-in rather
than user input.

Add more folks for feedback.

Please see my binutils post. For a .section directive with the same name but a different field. If the field is:

  • sh_flags or sh_type: warn
  • sh_link due to SHF_LINK_ORDER: no warning. produce separate sections
  • sh_entsize due to SHF_MERGE: still controversial

I agree with warning for sh_flags, I think there is too much legacy code out there that would behave differently.
I agree with sh_link producing separate sections. In an ideal world no-one writes this by hand in assembly.
For sh_entsize SHF_MERGE, this is informing the linker that it can produce an optimisation, which it is permitted to ignore. I tend towards an error message if it is implementable as it is a strong message to fix the assembler. The next best thing is a warning then clearing SHF_MERGE and setting sh_entsize to 0. I don't think a warning on its own is safe.

Feb 10 2020, 9:32 AM · Restricted Project

Feb 5 2020

bd1976llvm added a comment to D74006: [MC][ELF] Make linked-to symbol name part of ELFSectionKey.

Another thing is whether we should use sh_flags/sh_type to differentiate sections. In addition, whether we should use sh_entsize.
If yes, the implementation can be a bit clumsy. We may have to add many Elf*_Shdr fields to ELFSectionKey.
If no (GNU as behavior), we should add warnings. D73999 will do that.

The section group name is part of the key in both MC and GNU as. By analogy, SHF_LINK_ORDER+sh_link should probably be part of the key.

Field not specified by the .section directive (sh_addrline,sh_addr,sh_offset,sh_info) should definitely not be part of the key.

I have a slight preference for "no", though I don't currently have a particular good reason why "sh_entsize" should not be part of a key. Maybe we are fine with current .rodata.cst16 naming.

Feb 5 2020, 11:02 AM · Restricted Project
bd1976llvm added a comment to D74006: [MC][ELF] Make linked-to symbol name part of ELFSectionKey.

I think that this patch removes the need for uniquing sections for symbols with associated symbols which are explicitly assigned to a section name e.g. via attribute((section name)).

Feb 5 2020, 5:13 AM · Restricted Project
bd1976llvm added reviewers for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes: MaskRay, maskray0.
Feb 5 2020, 5:04 AM · Restricted Project
bd1976llvm updated the summary of D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Feb 5 2020, 5:04 AM · Restricted Project

Feb 4 2020

bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

updated the patch and description

Feb 4 2020, 6:40 PM · Restricted Project

Jan 23 2020

bd1976llvm added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Jan 23 2020, 6:49 PM · Restricted Project
bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

I have updated the patch. It now handles both "default" sections and "implicit" sections ("implicit" sections are those that are created by MC normally as it encounters symbols). I have improved the test coverage, organised the tests and improved the comments for the test cases. Sorry it took a while to update the patch.. this was a more difficult change than I had anticipated. I have tried a few different approaches, I am not entirely happy with the current implementation but it is difficult to simply further.

Jan 23 2020, 6:40 PM · Restricted Project

Jan 16 2020

bd1976llvm added a comment to D72197: [MC][ELF] Emit a relocation if target is defined in the same section and is non-local.
In D72197#1825390, @pcc wrote:
In D72197#1825012, @pcc wrote:

It looks like this change caused us to start rejecting:

.thumb
.thumb_func
.globl foo
.hidden foo
foo:
adr lr, foo + 1

with:

test.s:6:1: error: unsupported relocation on symbol
adr lr, foo + 1
^

This is exposed by this Android ART code: https://cs.android.com/android/platform/superproject/+/master:art/runtime/arch/arm/quick_entrypoints_arm.S;l=1826

I see a couple of possible fixes for this:

  1. We could go back to the previous behaviour for global hidden symbols.
  2. We could teach MC and LLD about the R_ARM_THM_ALU_PREL_11_0 relocation required to relocate this instruction. Arguably the ART code shouldn't be adding the 1 for Thumb here (because R_ARM_THM_ALU_PREL_11_0 adds the 1 itself), so ART would then need to be fixed.

foo is a hidden definition in the current object file. Can the ART code be changed to use a local symbol (adr lr, .Lfoo) instead?

It could, but I don't really see a good argument for local symbols having different behaviour to strong hidden symbols.

Not special casing hidden in the general case has the nice property that: the assembler behavior is not affected by the visibility. Only the binding can.

We can thus postpone decisions to the linker, and all the logic can be implemented on the linker side. Both a hidden defined symbol and a local symbol is considered non-preemptive, thus they have the same behavior at the linker stage.

The downside is that we increase object file sizes. Hidden symbols are not that common so the downside may be acceptable.

Jan 16 2020, 5:00 PM · Restricted Project
bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

I rebased the patch and addressed the review comments. I also noticed that there was a problem with the patch when explicitly placing non-mergeable sections into one of the "default" mergeable sections (e.g. .debug_str). As these "default" mergeable sections were not being added to the map it wasn't possible to explicitly place symbols into these even if the symbols entsize was compatible (as they would be put into a uniqued section instead). This is not acceptable for these "default" sections since for some of these like e.g. .debug_str, current binutils consumers make the assumption that there can only be one of these sections. The fix for this is WIP and I haven't added LIT tests for it yet; but, I have put up the patch in WIP state to get comments on the approach taken.

Jan 16 2020, 4:23 PM · Restricted Project
bd1976llvm added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Jan 16 2020, 3:03 AM · Restricted Project
bd1976llvm added a comment to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Is this worth merging into 10.0 if it lands after the branch?

Jan 16 2020, 2:45 AM · Restricted Project

Jan 14 2020

bd1976llvm added a comment to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Thanks, I'll address the points that you have raised.
I tried updating the assert and rebasing the patch but the assert is now firing in the It tests. I will have to investigate this tomorrow and update the patch then.

Jan 14 2020, 6:10 PM · Restricted Project
bd1976llvm added a comment to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

This is a partial fix for

Is this still considered a partial fix? Seems pretty complete to me, otherwise what's still missing?

Jan 14 2020, 12:21 PM · Restricted Project
bd1976llvm added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Jan 14 2020, 12:12 PM · Restricted Project

Jan 13 2020

bd1976llvm added a comment to D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.

I laid out a series of three options before:

  • Emit different object-file sections for different mergeability settings.
  • Only mark an object-file section as mergeable if all the symbols in it would have the same mergeability settings.
  • Stop implicitly using mergeability for "ordinary" sections (i.e. sections other than the string section).

Of the above, I really think #2 is the only complete solution.

Can you explain why you think #1 is not "complete"? All three seem to establish correctness; I can see how giving up on the optimization (#3) is not a great solution, but #1 doesn't have that problem and actually preserves it in more places. To be clear, this is just taking advantage of the ability to have multiple sections with the same name in an ELF object file; they will still be assembled into a single section in the linked image.

My understanding is that changing the flags on an MCSection retroactively is pretty counter to the architecture, so I'm not sure how we'd actually achieve #2.

Ah, ok, I reread https://reviews.llvm.org/D72194 and see that it's creating non-contiguous sections (option 1), with unique entity sizes. That should be fine. Dissenting opinion retracted. We should prefer https://reviews.llvm.org/D72194 with the commit message updated.

Jan 13 2020, 5:36 PM · Restricted Project
bd1976llvm abandoned D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.
Jan 13 2020, 5:36 PM · Restricted Project
bd1976llvm updated the diff for D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.

Updated the diff to address review comments and expand test coverage.

Jan 13 2020, 5:36 PM · Restricted Project
bd1976llvm added inline comments to D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Jan 13 2020, 5:28 PM · Restricted Project
bd1976llvm retitled D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes from WIP alternative approach for D68101 to [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Jan 13 2020, 5:18 PM · Restricted Project
bd1976llvm added a comment to D72197: [MC][ELF] Emit a relocation if target is defined in the same section and is non-local.

I wonder if is this really a bug or intentional. Clang and LLVM seem to ignore the possibility of symbol interposition (see: -fno-semantic-interposition). What is the LLVM policy for this?

Also, this code is missing a case for STB_GLOBAL and visibility != STV_DEFAULT. Actually, I have noticed before that this case is missing in various places in MC. As it stands this patch may cause a performance regression on platforms that use -fvisibility=hidden.

Visibility is irrelevant here. For a symbol defined in the same section as the instruction, the relocation record will reference a non-SECTION symbol instead of a STT_SECTION after this change. If the symbol is STV_HIDDEN, the linker will think the symbol is non-preemptible. So, no performance regression.

Jan 13 2020, 12:09 PM · Restricted Project

Jan 6 2020

bd1976llvm added a comment to D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.

Thanks for the feedback. I'm sorry that I am being slow on this... v.busy at the moment. Will try to finish the patch in the next few days.

Jan 6 2020, 7:47 PM · Restricted Project
bd1976llvm added a comment to D72197: [MC][ELF] Emit a relocation if target is defined in the same section and is non-local.

I wonder if is this really a bug or intentional. Clang and LLVM seem to ignore the possibility of symbol interposition (see: -fno-semantic-interposition). What is the LLVM policy for this?

Jan 6 2020, 2:08 AM · Restricted Project

Jan 3 2020

bd1976llvm added a comment to D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.

The solution described in that comment is not acceptable. We are not going to tell users that they cannot assign symbols explicitly to sections because LLVM is unable to promise not to miscompile if they do. It is LLVM's responsibility to correctly compile valid code; enabling mergeability for a section containing unnamed_addr symbols is an optimization, and if it is not safe, it needs to be disabled until we can figure out a way to make it safe.

Jan 3 2020, 6:58 PM · Restricted Project
bd1976llvm created D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes.
Jan 3 2020, 6:49 PM · Restricted Project

Jan 2 2020

bd1976llvm added a comment to D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.

Sorry for the delay in updating this.

Jan 2 2020, 4:59 PM · Restricted Project

Dec 3 2019

bd1976llvm added a comment to D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.

I can't speak for the pragma authors, but I strongly doubt the pragma is intended to force all affected globals to go into a single section unit, since the division into section units is purely an object-file representation issue.

Dec 3 2019, 2:29 AM · Restricted Project

Dec 2 2019

bd1976llvm added a comment to D70665: [llvm-readobj] - Implement --dependent-libraries flag..

The feature certainly could be implemented for COFF, so I think changing the name of the command line option is reasonable. The strings in the .deplibs sections map to libraries in an implementation defined manner, so I refer to them as "specifiers". In COFF dependent libraries are specified via "directives" in the object files. It might be worth naming the option something like: --dependent-lib-directives (as directives works for the entries in an ELF .deplibs sections equally as well as specifiers) ?

Dec 2 2019, 7:01 PM · Restricted Project
bd1976llvm added a comment to D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.

Given all that, this patch seems far too aggressive. While mergeable sections can be useful for optimizing arbitrary code that might not use a section, they are also extremely useful for optimizing the sorts of global tables that programmers frequently use explicit sections for. It seems to me that the right fix is to find the place that ensures that we don't put mergeable and non-mergeable objects in the same section unit (or at least conservatively makes the section unit non-mergeable) and fix it to consider entry size as well. That should be straightforward unless that place doesn't exist, in which case we have very serious problems.

Disagree (but I spent all day thinking about this). Going back to https://bugs.llvm.org/show_bug.cgi?id=31828, I think I solved the problem there incorrectly; we should be not marking the explicit section merge-able.

Dec 2 2019, 6:31 PM · Restricted Project

Dec 1 2019

bd1976llvm added a comment to D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.

Just to make sure I haven't missed anything, I would like to take this patch and run some numbers, for which I need a little bit of time.

Dec 1 2019, 8:18 AM · Restricted Project

Nov 21 2019

bd1976llvm added a comment to D65430: Add `--dependency-files` option, which is equivalent to compiler option -MD..

Hi All, Apologies that I missed the request for input here.

Nov 21 2019, 12:40 PM · Restricted Project

Oct 16 2019

bd1976llvm committed rL374997: Request access for bd1976llvm.
Request access for bd1976llvm
Oct 16 2019, 4:39 AM

Oct 2 2019

bd1976llvm added a reviewer for D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable: rjmccall.
Oct 2 2019, 10:48 AM · Restricted Project

Oct 1 2019

bd1976llvm added a reviewer for D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable: mcgrathr.
Oct 1 2019, 10:20 AM · Restricted Project
bd1976llvm updated subscribers of D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.

Hi Ben, this is not really my area of expertise, but it all starts to make some sense to me. I was expecting this transformation to happen earlier, but this is where the magic happens, and this probably where it belongs.
Just to make sure I haven't missed anything, I would like to take this patch and run some numbers, for which I need a little bit of time. If in the mean time someone with some more experience in this area here has a look too, that would be great....

Oct 1 2019, 10:20 AM · Restricted Project
bd1976llvm added a comment to D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.

Hi Sjoerd,

Oct 1 2019, 3:46 AM · Restricted Project

Sep 27 2019

bd1976llvm added a comment to D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.

This feels like it could cause a pretty serious regression. This essentially disables global merging with -fdata-sections, which I know at least one linker relies upon for code size.

Sep 27 2019, 11:05 AM · Restricted Project
bd1976llvm created D68147: [MC][ELF] Prevent globals with an explicit section from being mergeable by default and add a safe option to allow mergeable.
Sep 27 2019, 10:53 AM · Restricted Project, Restricted Project

Sep 26 2019

bd1976llvm created D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable.
Sep 26 2019, 1:08 PM · Restricted Project

Jun 12 2019

bd1976llvm committed rG52d3e4b4aa52: [Legacy LTO] Fix build bots: r363140: Fix export name (authored by bd1976llvm).
[Legacy LTO] Fix build bots: r363140: Fix export name
Jun 12 2019, 5:16 AM
bd1976llvm committed rL363151: [Legacy LTO] Fix build bots: r363140: Fix export name.
[Legacy LTO] Fix build bots: r363140: Fix export name
Jun 12 2019, 5:16 AM
bd1976llvm committed rG564d248ec2f2: [ThinLTO]LTO]Legacy] Fix dependent libraries support by adding querying of the… (authored by bd1976llvm).
[ThinLTO]LTO]Legacy] Fix dependent libraries support by adding querying of the…
Jun 12 2019, 4:06 AM
bd1976llvm committed rL363140: [ThinLTO]LTO]Legacy] Fix dependent libraries support by adding querying of the….
[ThinLTO]LTO]Legacy] Fix dependent libraries support by adding querying of the…
Jun 12 2019, 4:05 AM
bd1976llvm closed D62935: [ThinLTO][LTO][Legacy] Fix dependent libraries support by adding querying of the IRSymtab.
Jun 12 2019, 4:05 AM · Restricted Project

Jun 11 2019

bd1976llvm added a comment to D62935: [ThinLTO][LTO][Legacy] Fix dependent libraries support by adding querying of the IRSymtab.

Thanks. I was not sure if the new functions in LTOModule.h/.cpp were in the right file - but I don't see a better place to put them.

This is fine. Do you think you might want to query symbols from InputFile instead of LTOModule in the future?

Jun 11 2019, 1:03 AM · Restricted Project

Jun 10 2019

bd1976llvm added a comment to D62935: [ThinLTO][LTO][Legacy] Fix dependent libraries support by adding querying of the IRSymtab.

We should look at either merging the implementations or providing an api function that provides access to the "llvm.linker.options" named metadata via the IRSymtab. It may not be possible to merge the implementations because in COFF and MACHO the representation of dependent libraries is a string of command line options; whereas, the ELF representation is an array of library strings. This is because for COFF and MACHO there is basically only one linker for each file format so the command line format is fixed. For ELF there are a greater variety of linkers, so I just pass the library strings through to the linker, without processing them into command line arguments, and it is deferred to the linker to interpret the library strings.

It would be good if the API is flexible to return either. Not against having a different API to return linker options but I think that is less clean.

Jun 10 2019, 6:11 PM · Restricted Project

Jun 7 2019

bd1976llvm added a comment to D62935: [ThinLTO][LTO][Legacy] Fix dependent libraries support by adding querying of the IRSymtab.

Thanks for the response.

Jun 7 2019, 5:24 PM · Restricted Project
bd1976llvm retitled D62935: [ThinLTO][LTO][Legacy] Fix dependent libraries support by adding querying of the IRSymtab from [LTO][Legacy] Fix dependent libraries support by adding querying of the IRSymtab to [ThinLTO][LTO][Legacy] Fix dependent libraries support by adding querying of the IRSymtab.
Jun 7 2019, 2:43 AM · Restricted Project

Jun 5 2019

bd1976llvm created D62935: [ThinLTO][LTO][Legacy] Fix dependent libraries support by adding querying of the IRSymtab.
Jun 5 2019, 4:45 PM · Restricted Project

May 21 2019

bd1976llvm added a comment to D62090: [runtimes] Support ELF dependent libraries feature.

Great to see someone wanting to use this. During the RFC we decided not to set SHF_EXCLUDE on the .deplibs ELF section that is used to store the strings from these pragmas. This means that other linkers that don't support dependent libraries will not remove the sections and they will end up in the output. This shouldn't cause any functional issues; however, it might be noticed by developers and commented on. Perhaps we should change this decision? Also, have you considered the other option of having the build system invoke the clang with the --dependent-library=<specifier> switch. This is an alternative approach where modifying the source code is not appropriate, an example use is: https://reviews.llvm.org/D61742.

May 21 2019, 12:48 PM · Restricted Project

May 16 2019

bd1976llvm committed rL360984: [ELF] Implement Dependent Libraries Feature.
[ELF] Implement Dependent Libraries Feature
May 16 2019, 8:44 PM
bd1976llvm committed rG1d16515fb407: [ELF] Implement Dependent Libraries Feature (authored by bd1976llvm).
[ELF] Implement Dependent Libraries Feature
May 16 2019, 8:44 PM
bd1976llvm committed rC360984: [ELF] Implement Dependent Libraries Feature.
[ELF] Implement Dependent Libraries Feature
May 16 2019, 8:44 PM
bd1976llvm committed rLLD360984: [ELF] Implement Dependent Libraries Feature.
[ELF] Implement Dependent Libraries Feature
May 16 2019, 8:43 PM
bd1976llvm closed D60274: [ELF] Implement Dependent Libraries Feature.
May 16 2019, 8:43 PM · Restricted Project

May 9 2019

bd1976llvm committed rG3edca1ac1aea: [LLD][NFC] Refactor: BuildID hash size now computed in one place. (authored by bd1976llvm).
[LLD][NFC] Refactor: BuildID hash size now computed in one place.
May 9 2019, 1:07 AM
bd1976llvm committed rLLD360316: [LLD][NFC] Refactor: BuildID hash size now computed in one place..
[LLD][NFC] Refactor: BuildID hash size now computed in one place.
May 9 2019, 1:06 AM
bd1976llvm committed rL360316: [LLD][NFC] Refactor: BuildID hash size now computed in one place..
[LLD][NFC] Refactor: BuildID hash size now computed in one place.
May 9 2019, 1:06 AM
bd1976llvm closed D61078: [LLD][NFC] Refactor: BuildID hash size calculation is now done in a single place..
May 9 2019, 1:06 AM · Restricted Project

May 1 2019

bd1976llvm committed rG6e32dd6cfd0a: [LLD] Emit dynamic relocations for references to script symbols in -pie links (authored by bd1976llvm).
[LLD] Emit dynamic relocations for references to script symbols in -pie links
May 1 2019, 7:06 AM
bd1976llvm committed rLLD359683: [LLD] Emit dynamic relocations for references to script symbols in -pie links.
[LLD] Emit dynamic relocations for references to script symbols in -pie links
May 1 2019, 7:05 AM
bd1976llvm committed rL359683: [LLD] Emit dynamic relocations for references to script symbols in -pie links.
[LLD] Emit dynamic relocations for references to script symbols in -pie links
May 1 2019, 7:05 AM

Apr 30 2019

bd1976llvm added a comment to D55423: [LLD][ELF] - A fix for "linker script assignment loses relative nature of section" bug..

@grimar - this patch causes a regression in behaviour for references to script symbols. I think https://reviews.llvm.org/D61298 is a reasonable fix. Could you take a look?

Apr 30 2019, 2:52 AM · Restricted Project
bd1976llvm created D61298: [LLD] Emit dynamic relocations for references to script symbols in -pie links.
Apr 30 2019, 2:45 AM · Restricted Project

Apr 25 2019

bd1976llvm updated the diff for D61078: [LLD][NFC] Refactor: BuildID hash size calculation is now done in a single place..

Addressed review comments

Apr 25 2019, 5:30 PM · Restricted Project
bd1976llvm added a comment to D60274: [ELF] Implement Dependent Libraries Feature.

I am keen to keep this moving.

Apr 25 2019, 4:55 PM · Restricted Project

Apr 24 2019

bd1976llvm created D61078: [LLD][NFC] Refactor: BuildID hash size calculation is now done in a single place..
Apr 24 2019, 10:14 AM · Restricted Project

Apr 16 2019

bd1976llvm updated the diff for D60274: [ELF] Implement Dependent Libraries Feature.

No longer shortening "dependent libraries" to "deplibs" except for the .deplibs section (as this takes up bytes on disk).

Apr 16 2019, 5:54 PM · Restricted Project
bd1976llvm added inline comments to D60555: [llvm-objcopy] Fill .symtab_shndx section correctly.
Apr 16 2019, 10:28 AM · Restricted Project

Apr 15 2019

bd1976llvm added a comment to D60421: [ThinLTO] Fix ThinLTOCodegenerator to export llvm.used symbols.

LGTM! Thanks for waiting for me to confirm. Comments inline:

Apr 15 2019, 2:01 PM · Restricted Project

Apr 12 2019

bd1976llvm added a comment to D60421: [ThinLTO] Fix ThinLTOCodegenerator to export llvm.used symbols.

Sorry for the delay in replying. I was trying to understand if the upgrade path was important. Comments inline:

Apr 12 2019, 1:52 PM · Restricted Project

Apr 10 2019

bd1976llvm added a comment to D60421: [ThinLTO] Fix ThinLTOCodegenerator to export llvm.used symbols.

Hi Steven,

Apr 10 2019, 8:25 AM · Restricted Project

Apr 9 2019

bd1976llvm added inline comments to D60274: [ELF] Implement Dependent Libraries Feature.
Apr 9 2019, 11:07 AM · Restricted Project