[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

DiggerLin added a comment. In D142484#4080202 , @Jake-Egan wrote: > In D142484#4079909 , @jhuber6 wrote: > >> In D142484#4079869 , @Jake-Egan >> wrote: >> >>> Hi, this ne

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

Jake-Egan added a comment. In D142484#4079909 , @jhuber6 wrote: > In D142484#4079869 , @Jake-Egan > wrote: > >> Hi, this new test fails on AIX. Could you take a look? >> https://lab.llvm.org/buildbot/#/builders/2

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 added a comment. In D142484#4079869 , @Jake-Egan wrote: > Hi, this new test fails on AIX. Could you take a look? > https://lab.llvm.org/buildbot/#/builders/214/builds/5477/steps/6/logs/FAIL__Clang__linker-wrapper-libs_c I might just put that this

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 added a comment. In D142484#4079869 , @Jake-Egan wrote: > Hi, this new test fails on AIX. Could you take a look? > https://lab.llvm.org/buildbot/#/builders/214/builds/5477/steps/6/logs/FAIL__Clang__linker-wrapper-libs_c I'm actually not sure why

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

Jake-Egan added a comment. Hi, this new test fails on AIX. Could you take a look? https://lab.llvm.org/buildbot/#/builders/214/builds/5477/steps/6/logs/FAIL__Clang__linker-wrapper-libs_c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142484/new/ htt

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. jhuber6 marked an inline comment as done. Closed by commit rG1964c334782e: [LinkerWrapper] Only import static libraries with needed symbols (authored by jhuber6). Chan

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

tra added a comment. > I'm somewhat hoping to get this in before the fork that happens in a few > hours. OK. We can fix it later if it turns out that we've missed something. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142484/new/ https://review

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 marked an inline comment as done. jhuber6 added a comment. In D142484#4078377 , @tra wrote: > LGTM. Please wait a bit before landing it, in case @MaskRay has something to > say. I'm somewhat hoping to get this in before the fork that happens in

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. LGTM. Please wait a bit before landing it, in case @MaskRay has something to say. Comment at: clang/test/Driver/linker-wrapper-libs.c:38-39 + +// LIBRARY-RESOLVES: clang{{.*}} -o

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 updated this revision to Diff 491883. jhuber6 added a comment. Adding test and making the logic a bit more readable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142484/new/ https://reviews.llvm.org/D142484 Files: clang/test/Driver/link

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 marked an inline comment as done. jhuber6 added a comment. In D142484#4077979 , @tra wrote: > We could also use more test cases. E.g. weak symbols (should not cause object > extraction) Yeah, I'll try to add a reasonable test here. It's a little

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

tra added a comment. We could also use more test cases. E.g. weak symbols (should not cause object extraction) Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1197 + +Resolved |= ((OldSym & Sym_Undefined && !(OldSym & Sym_Weak)) && + !(*

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1217-1218 +/// 1) It defines an undefined symbol in a regular object filie. +/// 2) It defines a global symbol without hidden visibility that has not +/// yet been defined.

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

tra added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1217-1218 +/// 1) It defines an undefined symbol in a regular object filie. +/// 2) It defines a global symbol without hidden visibility that has not +/// yet been defined. +Ex

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 added a comment. In D142484#4077811 , @tra wrote: > @MaskRay - we seem to be reinventing the linker here and could use your > expertise. Yeah, this reinvents a lot of logic. But I don't think there's an easy way to get around this without dupli

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

tra added a subscriber: MaskRay. tra added a comment. @MaskRay - we seem to be reinventing the linker here and could use your expertise. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1217-1218 +/// 1) It defines an undefined symbol in a regular object f

[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, tianshilei1992, JonChesterfield, tra, yaxunl. Herald added a subscriber: kosarev. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: openmp-commits, cfe-commits, sstefan1. Herald added