Page MenuHomePhabricator

lldProject
ActivePublic

Details

Description

LLVM Linker

Recent Activity

Yesterday

mstorsjo added a comment to D55293: [LLD][COFF] Unmerged sections.
In D55293#1328930, @rnk wrote:

Yep, I looked at removing this before. It might actually be worth it for mingw, because GCC outputs sections named like ".text$_Z3foov", so you can imagine that this map is going to have one entry per symbol, so one extra memory allocation per symbol.

@mstorsjo, I assume that ld.bfd doesn't actually sort the .text (and .pdata and .xdata) section contributions alphabetically. Should we drop the "$_Z3foov" portion of the object file section name before making the unmerged section? It might matter for performance.

Wed, Dec 12, 11:43 PM · lld
rnk added a comment to D55293: [LLD][COFF] Unmerged sections.

I just hit send before saying that, at a high level, looks good, but we should check with @ruiu and @pcc about the terminology.

Wed, Dec 12, 3:09 PM · lld
rnk added reviewers for D55293: [LLD][COFF] Unmerged sections: pcc, ruiu.
Wed, Dec 12, 3:09 PM · lld
rnk updated subscribers of D55293: [LLD][COFF] Unmerged sections.

I used a std::map below instead of a DenseHash because we need to retain a sorted order when enumerating.

Wed, Dec 12, 3:09 PM · lld

Tue, Dec 11

atanasyan updated the diff for D40147: [MIPS] Handle cross-mode (regular <-> microMIPS) jumps.
  • Remove new argument from relocateOne methods
  • Introduce new helper function setMicroMipsBit and call it before the relocateOne
Tue, Dec 11, 2:04 PM · lld

Fri, Dec 7

krytarowski resigned from D55443: [test] Capture stderr from 'tar --version' call as well.

I don't feel enough comfortable with this Python code.

Fri, Dec 7, 3:39 PM · lld
mgorny added inline comments to D55443: [test] Capture stderr from 'tar --version' call as well.
Fri, Dec 7, 1:00 PM · lld
ruiu added inline comments to D55443: [test] Capture stderr from 'tar --version' call as well.
Fri, Dec 7, 12:43 PM · lld
mgorny updated the diff for D55443: [test] Capture stderr from 'tar --version' call as well.
Fri, Dec 7, 11:38 AM · lld
MaskRay added inline comments to D55443: [test] Capture stderr from 'tar --version' call as well.
Fri, Dec 7, 11:19 AM · lld
ruiu added inline comments to D55443: [test] Capture stderr from 'tar --version' call as well.
Fri, Dec 7, 11:08 AM · lld
mgorny added a comment to D55443: [test] Capture stderr from 'tar --version' call as well.

Sorry, it seems that my reply didn't go through.

Fri, Dec 7, 11:03 AM · lld
ruiu added inline comments to D55443: [test] Capture stderr from 'tar --version' call as well.
Fri, Dec 7, 11:00 AM · lld
mgorny closed D55441: [lld] [test] Fix reproduce-blackslash.s test with NetBSD tar.
Fri, Dec 7, 11:00 AM · lld
mgorny updated the diff for D55443: [test] Capture stderr from 'tar --version' call as well.
Fri, Dec 7, 10:51 AM · lld
krytarowski added a comment to D55441: [lld] [test] Fix reproduce-blackslash.s test with NetBSD tar.

bsdtar is libarchive (by BSD people too)

Fri, Dec 7, 10:40 AM · lld
ruiu accepted D55441: [lld] [test] Fix reproduce-blackslash.s test with NetBSD tar.

We might be able to change NetBSD's tar but I guess that takes very long time. This test is not very important in the sense that this tests just test a corner case. So I think we should just land this to make it work on NetBSD.

Fri, Dec 7, 10:37 AM · lld
ruiu added a comment to D55441: [lld] [test] Fix reproduce-blackslash.s test with NetBSD tar.

I thought NetBSD's tar is bsdtar because it's BSD... Anyways, I think I'm fine with this change, as the new test (which matches both foo\\.o and foo\.o) does not match a string that we don't want it to match.

Fri, Dec 7, 10:27 AM · lld
krytarowski added a comment to D55441: [lld] [test] Fix reproduce-blackslash.s test with NetBSD tar.

In my opinion it's better to set UNSUPPORTED and align the NetBSD tar to GNU tar and libarchive.

Fri, Dec 7, 10:23 AM · lld
MaskRay added inline comments to D55443: [test] Capture stderr from 'tar --version' call as well.
Fri, Dec 7, 9:52 AM · lld
mgorny created D55443: [test] Capture stderr from 'tar --version' call as well.
Fri, Dec 7, 9:20 AM · lld
mgorny created D55441: [lld] [test] Fix reproduce-blackslash.s test with NetBSD tar.
Fri, Dec 7, 9:12 AM · lld

Tue, Dec 4

ruiu added a comment to D54747: Discard debuginfo for object files empty after GC.

It seems to me that just adding --start-lib to his command line can fix the issue, so I'm waiting for Robert's response. If it doesn't work for some reason, we can analyze why it doesn't work and then discuss what we can do for his problem.

Tue, Dec 4, 4:21 PM · lld
echristo added a comment to D54747: Discard debuginfo for object files empty after GC.

But 2GB is perhaps still too big and I guess a large part of it can be for dead sections. If we fix this, I'd like to fix it in a proper way so that we can completely eliminate debug info for dead sections.

Tue, Dec 4, 4:18 PM · lld
aganea created D55293: [LLD][COFF] Unmerged sections.
Tue, Dec 4, 1:08 PM · lld
ruiu added inline comments to D54868: [PPC][PPC64] PPC_REL14 and PPC64_REL14 relocations.
Tue, Dec 4, 11:02 AM · lld
aganea planned changes to D49366: Add support for /FUNCTIONPADMIN command-line option.

Just for the record, this is not as simple as I initially thought. We should only be padding functions that originate from hotpatch-enabled OBJs. Currently, LLVM currently does not generate /hotpatch OBJs (at least the flag isn't set). In theory, x64 and ARM OBJs should be fine to have this flag enabled. I'll do more research and get back.

Tue, Dec 4, 9:25 AM · lld
martell closed D54871: [ELF] Allow discarding of .rela.plt.
Tue, Dec 4, 4:40 AM · lld
grimar added a comment to D54871: [ELF] Allow discarding of .rela.plt.

Sorry only getting around to this comment now. I think this is probably what you are looking for
https://github.com/torvalds/linux/blob/master/arch/powerpc/kernel/vmlinux.lds.S#L387

Tue, Dec 4, 4:38 AM · lld
martell added a comment to D54871: [ELF] Allow discarding of .rela.plt.

Sorry only getting around to this comment now. I think this is probably what you are looking for
https://github.com/torvalds/linux/blob/master/arch/powerpc/kernel/vmlinux.lds.S#L387

Tue, Dec 4, 4:36 AM · lld
martell closed D54868: [PPC][PPC64] PPC_REL14 and PPC64_REL14 relocations.
Tue, Dec 4, 4:30 AM · lld

Mon, Dec 3

ruiu added inline comments to D54868: [PPC][PPC64] PPC_REL14 and PPC64_REL14 relocations.
Mon, Dec 3, 8:31 AM · lld
sfertile accepted D54868: [PPC][PPC64] PPC_REL14 and PPC64_REL14 relocations.

Other then my one comment it LGTM.

Mon, Dec 3, 7:00 AM · lld

Sun, Dec 2

martell added a comment to D54868: [PPC][PPC64] PPC_REL14 and PPC64_REL14 relocations.

This probably needs another approval based on the small changes I made to address the warning

Sun, Dec 2, 3:54 PM · lld
martell updated the diff for D54868: [PPC][PPC64] PPC_REL14 and PPC64_REL14 relocations.

update to address comments

Sun, Dec 2, 3:53 PM · lld
martell updated the diff for D54868: [PPC][PPC64] PPC_REL14 and PPC64_REL14 relocations.
Sun, Dec 2, 3:52 PM · lld

Fri, Nov 30

ruiu added a comment to D54747: Discard debuginfo for object files empty after GC.

But 2GB is perhaps still too big and I guess a large part of it can be for dead sections. If we fix this, I'd like to fix it in a proper way so that we can completely eliminate debug info for dead sections.

Fri, Nov 30, 2:04 PM · lld
aganea abandoned D46427: [PDB] Quote linker arguments containing spaces (mimic MSVC).
Fri, Nov 30, 8:41 AM · lld
aganea commandeered D46427: [PDB] Quote linker arguments containing spaces (mimic MSVC).

Commited as rL348001: [PDB] Quote linker arguments containing spaces (mimic MSVC)

Fri, Nov 30, 8:41 AM · lld
aganea commandeered D49366: Add support for /FUNCTIONPADMIN command-line option.

After discussing ofline with Stefan, I will take ownership and will finish this patch.

Fri, Nov 30, 6:01 AM · lld

Wed, Nov 28

echristo added a comment to D54747: Discard debuginfo for object files empty after GC.

Thank you for the patch.

What you are doing in this patch is not too complicated and makes sense to me. That said, if actual size saving is not significant as you said in https://github.com/rust-lang/rust/issues/56068#issuecomment-440160568, it may not be worth doing. It seems to me that if debug info is already 2.4GB, shrinking it to 2GB doesn't make much difference. Do you have more convincing examples?

Wed, Nov 28, 10:38 PM · lld
MaskRay added a comment to D54747: Discard debuginfo for object files empty after GC.

If we decide to optimize DWARF garbage collection, something generic will be cool.

Wed, Nov 28, 4:00 PM · lld
ruiu added a comment to D54747: Discard debuginfo for object files empty after GC.

Thank you for the patch.

Wed, Nov 28, 2:57 PM · lld
atanasyan closed D54920: [ELF][MIPS] Handle mips in the OUTPUT_FORMAT directive.
Wed, Nov 28, 3:44 AM · lld
grimar accepted D54920: [ELF][MIPS] Handle mips in the OUTPUT_FORMAT directive.

LGTM.

Wed, Nov 28, 3:16 AM · lld

Tue, Nov 27

atanasyan updated the diff for D54920: [ELF][MIPS] Handle mips in the OUTPUT_FORMAT directive.
  • Simplify handling of the readBfdName return value
Tue, Nov 27, 10:05 PM · lld
joel added inline comments to D54868: [PPC][PPC64] PPC_REL14 and PPC64_REL14 relocations.
Tue, Nov 27, 3:32 PM · lld
ruiu added a comment to D54920: [ELF][MIPS] Handle mips in the OUTPUT_FORMAT directive.

As to defining a struct to store the parse result of parseBfdName, I don't think we need to define a new struct. That seems a bit too heavyweight to represent just a temporary value.

Tue, Nov 27, 9:31 AM · lld
ruiu added a comment to D54920: [ELF][MIPS] Handle mips in the OUTPUT_FORMAT directive.
  • Return a boolean value indicating whether the name is MIPS N32 ABI or not from the readBfdName .

    I think one of the problem is that there are multiple really different architectures and ABIs under the same EM_MIPS name. In particular, maybe it was better to introduce two different names EM_MIPS and EM_MIPS_64 many years ago. In that case ntradmips (N32 ABI) could be describes like {ELF32LEKind, EM_MIPS_64}.
Tue, Nov 27, 9:29 AM · lld
ruiu added a comment to D40147: [MIPS] Handle cross-mode (regular <-> microMIPS) jumps.

Adding a new parameter to relocateOne doesn't look beautiful, but that makes me think whether you are editing the right place of the code. relocateOne is supposed to apply a single relocation based on a value passed to the function. No complex logic should be in the function. Passing one more parameter so that you can do more complex computation in the function may not be a good idea.

That's interesting - you suggested to pass a Symbol to relocateOne on Feb 16.
https://reviews.llvm.org/D40147#inline-379274

I will try to implement an alternative solution. Maybe by updating one of initial patches.

Tue, Nov 27, 9:11 AM · lld