lldProject
ActivePublic

Details

Description

LLVM Linker

Recent Activity

Wed, Oct 3

sfertile closed D52564: [PPC64] Add test for toc-restore on recursive call..
Wed, Oct 3, 3:02 PM · lld
aganea added a comment to D46427: [PDB] Quote linker arguments containing spaces (mimic MSVC).

@lantictac The following works in the test:

RUN: lld-link %S/a.obj /entry:"1 "'"'hello'"'" 2"

Outputs as expected:

lld-link: error: <root>: undefined symbol: 1 "hello" 2
Wed, Oct 3, 10:57 AM · lld

Tue, Oct 2

zturner closed D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.
Tue, Oct 2, 12:44 PM · lld
lantictac added a comment to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

Thanks for the clarification. Is there any way to manually close the review?

Tue, Oct 2, 12:30 PM · lld
zturner updated subscribers of D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

Ahh I see. You need to say "Differential Revision:
https://reviews.llvm.org/D50404" in the commit message. just the URL is
not sufficient to trigger the magic auto-close.

Tue, Oct 2, 12:10 PM · lld
lantictac added a comment to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

I committed this as r342894 but for some reason the review didn't complete. Apologies, the last LLVM commits I did were pre-phabricator so it seems I messed up somewhere. Any pointers appreciated.

Tue, Oct 2, 12:04 PM · lld
lantictac added 2 commit(s) for D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}: rLLD342894: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash…, rL342894: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash….
Tue, Oct 2, 12:00 PM · lld
zturner added a comment to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

It would be great if we could land this soon-ish. I have a lot of people complaining that lld-link rejects /DEBUG:FASTLINK when using the VS extension. Is there any work left to be done or can we land this?

Tue, Oct 2, 11:42 AM · lld

Fri, Sep 28

sfertile added inline comments to D52564: [PPC64] Add test for toc-restore on recursive call..
Fri, Sep 28, 10:10 AM · lld

Wed, Sep 26

MaskRay added inline comments to D52564: [PPC64] Add test for toc-restore on recursive call..
Wed, Sep 26, 2:35 PM · lld
ruiu accepted D52564: [PPC64] Add test for toc-restore on recursive call..

LGTM

Wed, Sep 26, 2:19 PM · lld
MaskRay accepted D52564: [PPC64] Add test for toc-restore on recursive call..

LG (but please wait for the code owner).

Wed, Sep 26, 2:19 PM · lld
sfertile added inline comments to D52564: [PPC64] Add test for toc-restore on recursive call..
Wed, Sep 26, 1:47 PM · lld
MaskRay added inline comments to D52564: [PPC64] Add test for toc-restore on recursive call..
Wed, Sep 26, 11:29 AM · lld
sfertile created D52564: [PPC64] Add test for toc-restore on recursive call..
Wed, Sep 26, 11:10 AM · lld

Thu, Sep 20

ikudrin added a comment to D50560: [LLD] Enable Visual Studio compatible diagnostics..

With overloaded operators we can write something like

error("relocation " + toString(Type) +
        " cannot refer to absolute symbol: " + toString(Sym) +
        definedAt(toObjLoc(Sym.File)) +
        referencedBy(S.getSymLoc(Sym, RelOff)));

How would you prefer it to look like?
I don't insist on the current style, but I believe it is important to be discussed prior to rewriting the code because the decision will influence the design directly.

Thu, Sep 20, 7:00 PM · lld
ruiu accepted D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

LGTM

Thu, Sep 20, 3:04 PM · lld
lantictac updated the diff for D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

Updated patch with ruiu's suggestions. Also added a test for the warning emitted when an unknown /debugtype option is used.

Thu, Sep 20, 2:57 PM · lld
ruiu added a comment to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

Looks much better!

Thu, Sep 20, 10:09 AM · lld
lantictac updated the diff for D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

Updated patch based on suggestions from ruiu

Thu, Sep 20, 10:05 AM · lld
ruiu added a comment to D50560: [LLD] Enable Visual Studio compatible diagnostics..

Operator overriding can always be rewritten as function calls, so instead of operator+(X), you can always define something like add(X)?

Thu, Sep 20, 9:31 AM · lld
ruiu added inline comments to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.
Thu, Sep 20, 9:11 AM · lld
lantictac updated the diff for D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

Moved/inlined parsing and diagnostic code into the specialized parse functions to (hopefully) simplify the calls in LinkerDriver::link().

Thu, Sep 20, 8:58 AM · lld
ikudrin added a comment to D50560: [LLD] Enable Visual Studio compatible diagnostics..

@chrisjackson @arichardson @ruiu, thank you for the feedback! I am going to continue working on the patch somewhere in October. I would really appreciate any additional comments meanwhile.

Thu, Sep 20, 12:38 AM · lld

Wed, Sep 19

ruiu added a comment to D50560: [LLD] Enable Visual Studio compatible diagnostics..

I haven't read Diagnostics.cpp to focus on the ABI itself, but it looks like this approach is better than https://reviews.llvm.org/D47540, though this patch seems a bit too abstracted and too object-oriented to my taste. Can you add comment to the code? Also, please avoid operator overriding and eliminate Diag class (you can do that because all you need is a boolean flag indicating whether VS output is wanted or not.)

Wed, Sep 19, 2:08 PM · lld
chrisjackson added a comment to D50560: [LLD] Enable Visual Studio compatible diagnostics..

@ruiu
Hello Rui, do you have any thoughts on this implementation compared to D47540 ?

Wed, Sep 19, 10:19 AM · lld

Sep 13 2018

ruiu closed D50598: LLD COFF: Add support for /force:multiple option.
Sep 13 2018, 3:06 PM · lld
ruiu added a comment to D50598: LLD COFF: Add support for /force:multiple option.

It needs a test, but I'll write that since the test should be small.

Sep 13 2018, 2:59 PM · lld
ruiu added a comment to D50598: LLD COFF: Add support for /force:multiple option.

If you have a commit access, you can do that yourself, but since you don't have one, I'll do that for you.

Sep 13 2018, 2:55 PM · lld
troughton accepted D50598: LLD COFF: Add support for /force:multiple option.

If this looks fine, could you please merge it in? I’m not sure how the merge process is handled for LLVM.

Sep 13 2018, 2:54 PM · lld

Sep 12 2018

ruiu added inline comments to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.
Sep 12 2018, 8:45 AM · lld
lantictac added a comment to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

Ping

Sep 12 2018, 8:36 AM · lld

Sep 10 2018

phosek added a comment to D51833: ELF: Add --build-id-link-dir=DIR switch.

I think I don't understand the use case of the feature yet.

  • If you hard-link two files, they have the identical contents (strictly speaking there is only one file with two filenames). If a debugger can find an executable having debug info in .build-id/xx/xxxxx directory, it should be able to find it in the executable that's being debugged. So, how does it work?
Sep 10 2018, 5:18 PM · lld
ruiu added a comment to D51833: ELF: Add --build-id-link-dir=DIR switch.

I think I don't understand the use case of the feature yet.

Sep 10 2018, 4:59 PM · lld
phosek updated the diff for D51833: ELF: Add --build-id-link-dir=DIR switch.
Sep 10 2018, 4:46 PM · lld
phosek commandeered D51833: ELF: Add --build-id-link-dir=DIR switch.
Sep 10 2018, 4:45 PM · lld
phosek added a comment to D51833: ELF: Add --build-id-link-dir=DIR switch.

This seems like a new feature proposal, and we haven't discussed this before. It's not clear to me why you have to do this inside the linker rather than a post-processing tool. Could you please elaborate about why you want to add a new option?

Sep 10 2018, 4:43 PM · lld
troughton updated the diff for D50598: LLD COFF: Add support for /force:multiple option.

Specify std::string over auto and change message to Msg.

Sep 10 2018, 3:37 PM · lld
ruiu added a comment to D51833: ELF: Add --build-id-link-dir=DIR switch.

This seems like a new feature proposal, and we haven't discussed this before. It's not clear to me why you have to do this inside the linker rather than a post-processing tool. Could you please elaborate about why you want to add a new option?

Sep 10 2018, 9:05 AM · lld
ruiu accepted D50598: LLD COFF: Add support for /force:multiple option.

LGTM

Sep 10 2018, 8:10 AM · lld

Sep 8 2018

mcgrathr created D51833: ELF: Add --build-id-link-dir=DIR switch.
Sep 8 2018, 3:20 PM · lld

Sep 7 2018

troughton updated the diff for D50598: LLD COFF: Add support for /force:multiple option.

Address review comments by inlining errorOrWarnMultiple.

Sep 7 2018, 4:52 PM · lld
arichardson added inline comments to D50560: [LLD] Enable Visual Studio compatible diagnostics..
Sep 7 2018, 6:29 AM · lld
chrisjackson added a comment to D50560: [LLD] Enable Visual Studio compatible diagnostics..

I have experimented with this implementation and I agree that separating the diagnostic location functions into a distinct library is a good structured approach. I think this would be a reasonable and more explicit support for Visual Studio Diagnostics than D47540 which really just shoehorned in support.

Sep 7 2018, 5:21 AM · lld

Sep 5 2018

MaskRay added a comment to D51607: [ELF] Fix bugzilla #38748 for option no-rosegment.

I made a comment in https://bugs.llvm.org/show_bug.cgi?id=38784

Sep 5 2018, 5:59 PM · lld
smeenai added a reviewer for D51607: [ELF] Fix bugzilla #38748 for option no-rosegment: ruiu.

You'll wanna upload the patch with full context (http://llvm.org/docs/Phabricator.html).

Sep 5 2018, 4:32 PM · lld
NickHung updated the diff for D51607: [ELF] Fix bugzilla #38748 for option no-rosegment.
Sep 5 2018, 4:20 PM · lld
NickHung added a project to D51607: [ELF] Fix bugzilla #38748 for option no-rosegment: lld.
Sep 5 2018, 4:18 PM · lld
lantictac added inline comments to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.
Sep 5 2018, 9:18 AM · lld
lantictac updated the diff for D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

Renamed DebugGeneration enum (name was based on MSDN description of /debug argument) to DebugKind. Flattened the argument handling code to be a little clearer.

Sep 5 2018, 9:13 AM · lld