[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-09-05 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In https://reviews.llvm.org/D51440#1225318, @steven_wu wrote: > I do prefer the current approach especially on Darwin. Some concerns of > switching to use "-L + -l" are: > > 1. clang and compiler-rt are rev-locked. Inferring the compiler-rt from > resource-dir and pa

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-09-05 Thread Steven Wu via Phabricator via cfe-commits
steven_wu added a comment. I do prefer the current approach especially on Darwin. Some concerns of switching to use "-L + -l" are: 1. clang and compiler-rt are rev-locked. Inferring the compiler-rt from resource-dir and passing to linker with the full path can prevent mistakes of mixing them u

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-09-04 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a subscriber: dexonsmith. beanz added a comment. Unfortunately I can't make this kind of policy decision about whether or not this would be acceptable for Darwin. That would probably need to be @dexonsmith. Repository: rC Clang https://reviews.llvm.org/D51440 _

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-08-31 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In https://reviews.llvm.org/D51440#1220925, @phosek wrote: > In https://reviews.llvm.org/D51440#1218859, @mstorsjo wrote: > > > I'll see if I can get to looking at that sometime soon. I had this patch > > lying around as an attempt to work around the libtool issue in >

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-08-31 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. In https://reviews.llvm.org/D51440#1218859, @mstorsjo wrote: > I'll see if I can get to looking at that sometime soon. I had this patch > lying around as an attempt to work around the libtool issue in > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27866 which doesn't se

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-08-30 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In https://reviews.llvm.org/D51440#1218817, @phosek wrote: > What about other compiler-rt runtimes? Shouldn't we use the same approach for > those? Yes, I guess so. > In case of multiarch runtime layout, we already add >

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-08-30 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. What about other compiler-rt runtimes? Shouldn't we use the same approach for those? In case of multiarch runtime layout, we already add the runtime directory to the new `LibraryPaths` list,

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-08-29 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment. In https://reviews.llvm.org/D51440#1217910, @mstorsjo wrote: > In https://reviews.llvm.org/D51440#1217839, @manojgupta wrote: > > > Just a minor comment regarding test cases: Since you are adding both > > -L/path/ and -l, the test cases should be updated to check fo

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-08-29 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In https://reviews.llvm.org/D51440#1217839, @manojgupta wrote: > Just a minor comment regarding test cases: Since you are adding both -L/path/ > and -l, the test cases should be updated to check for the -L/path/ > argument as well. I guess I could do that, although

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-08-29 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Since the libraries will no longer be specified with their full path, how will you know that the **right** library will be picked, the one compiler intended? Repository: rC Clang https://reviews.llvm.org/D51440 ___ cf

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-08-29 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment. Just a minor comment regarding test cases: Since you are adding both -L/path/ and -l, the test cases should be updated to check for the -L/path/ argument as well. Repository: rC Clang https://reviews.llvm.org/D51440 __

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-08-29 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a reviewer: phosek. beanz added a subscriber: phosek. beanz added a comment. Looping in @phosek. Repository: rC Clang https://reviews.llvm.org/D51440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

2018-08-29 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo created this revision. mstorsjo added reviewers: manojgupta, chandlerc, beanz. Herald added subscribers: aheejin, sbc100, dberris, srhines. Herald added a reviewer: javed.absar. This avoids a libtool issue (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27866), where libtool fails to pic