[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

This revision was automatically updated to reflect the committed changes. Closed by commit rLLD360984: [ELF] Implement Dependent Libraries Feature (authored by bd1976llvm, committed by ). Changed prior to commit: https://reviews.llvm.org/D60274?vs=195495&id=199972#toc Repository: rLLD LLVM L

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

phosek added a comment. @bd1976llvm do you plan on landing this? We'd really like to start using this feature. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60274/new/ https://reviews.llvm.org/D60274 ___ cfe-commits mailing list cfe-commit

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

ruiu accepted this revision. ruiu added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60274/new/ https://reviews.llvm.org/D60274 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

ruiu added a comment. I don't have any particular comment, and I'll give an LGTM soon as people seem to generally happy about this. If you are not happy about this patch or the feature itself, please shout. This feature is about to land. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

bd1976llvm added a comment. I am keen to keep this moving. I think there are a few things outstanding: 1. Need to resolve concerns w.r.t the design. 2. I need to find out whether I should be doing validation when reading the metadata in LLVM. 3. The LLD side needs a more detailed review. @jykn

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

probinson added a comment. I'm okay with the PS4-specific bits. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60274/new/ https://reviews.llvm.org/D60274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

bd1976llvm updated this revision to Diff 195495. bd1976llvm added a comment. No longer shortening "dependent libraries" to "deplibs" except for the .deplibs section (as this takes up bytes on disk). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60274/new/ https://reviews.llvm.org/D6027

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

compnerd added inline comments. Comment at: lld/ELF/InputFiles.cpp:662 } + case SHT_LLVM_DEPLIBS: { +if (Config->Relocatable) Can you make the flag here reflect the name as well? (`SHT_LLVM_DEPENDENT_LIBRARIES`) Comment at: lld/ELF/Op

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

bd1976llvm marked 5 inline comments as done. bd1976llvm added inline comments. Comment at: lld/ELF/InputFiles.cpp:402 + if (Config->DepLibs) +if (fs::exists(Specifier)) + Driver->addFile(Specifier, /*WithLOption=*/false); jyknight wrote: > bd1976llvm wr

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

jyknight added inline comments. Comment at: lld/ELF/InputFiles.cpp:402 + if (Config->DepLibs) +if (fs::exists(Specifier)) + Driver->addFile(Specifier, /*WithLOption=*/false); bd1976llvm wrote: > jyknight wrote: > > bd1976llvm wrote: > > > jyknight wrote

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

bd1976llvm marked 25 inline comments as done. bd1976llvm added a comment. I have updated the diff to address review comments. I think we can continue to refine this patch in parallel with discussing the design further (I am not dismissing the concerns raised by @compnerd and @jyknight). I am un