Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-23 Thread Andrew Ford via cfe-commits
andrewford added a subscriber: andrewford. andrewford added a comment. This is breaking our android lldb build, because it uses std::to_string. Looks like there is llvm::to_string, which should be preferred. Would someone mind changing it? I don't have commit access or I would myself. :) Than

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-23 Thread Teresa Johnson via cfe-commits
On Fri, Sep 23, 2016, 4:38 PM Andrew Ford wrote: > andrewford added a subscriber: andrewford. > andrewford added a comment. > > This is breaking our android lldb build, because it uses std::to_string. > Looks like there is llvm::to_string, which should be preferred. Would > someone mind changing

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-23 Thread Teresa Johnson via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL282291: [LTO] Add -flto-jobs=N to control backend parallelism (authored by tejohnson). Changed prior to commit: https://reviews.llvm.org/D24826?vs=72317&id=72349#toc Repository: rL LLVM https://revi

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-23 Thread Mehdi AMINI via cfe-commits
mehdi_amini accepted this revision. mehdi_amini added a comment. This revision is now accepted and ready to land. LGTM, Thanks! https://reviews.llvm.org/D24826 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-23 Thread Teresa Johnson via cfe-commits
tejohnson updated this revision to Diff 72317. tejohnson added a comment. Update option help message per Mehdi's suggestion https://reviews.llvm.org/D24826 Files: include/clang/Driver/Options.td lib/Driver/Tools.cpp test/Driver/lto-jobs.c Index: test/Driver/lto-jobs.c ===

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-23 Thread Mehdi AMINI via cfe-commits
mehdi_amini added inline comments. Comment at: include/clang/Driver/Options.td:818 @@ -815,1 +817,3 @@ + HelpText<"Controls the backend parallelism of -flto=thin (default " + "of 0 means use std::thread::hardware_concurrency)">; def fthinlto_index_EQ : Joined<["-"], "f

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-23 Thread Teresa Johnson via cfe-commits
tejohnson added a comment. > > I do see other uses of -mllvm in lib/Driver/Tools.cpp, but are you talking > > about something else? > > I think this is okay, since clang is talking to the same version of > libLTO.dylib. I feel like there might be another case where > clang talks to libLTO

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-23 Thread Teresa Johnson via cfe-commits
tejohnson updated this revision to Diff 72301. tejohnson added a comment. Update option description as per decision to split from parallel code gen. https://reviews.llvm.org/D24826 Files: include/clang/Driver/Options.td lib/Driver/Tools.cpp test/Driver/lto-jobs.c Index: test/Driver/lto-j

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-22 Thread Duncan P. N. Exon Smith via cfe-commits
> On 2016-Sep-22, at 09:53, Teresa Johnson wrote: > > tejohnson added a comment. > > In https://reviews.llvm.org/D24826#549788, @mehdi_amini wrote: > >> The Gold path looks fine. >> On OSX, we would have the clang driver relying on a LLVM cl::opt, for which >> I don't think there is any prec

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-22 Thread Teresa Johnson via cfe-commits
tejohnson added a comment. In https://reviews.llvm.org/D24826#549788, @mehdi_amini wrote: > The Gold path looks fine. > On OSX, we would have the clang driver relying on a LLVM cl::opt, for which > I don't think there is any precedent. CC Duncan for advice. I do see other uses of -mllvm in l

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-22 Thread Mehdi AMINI via cfe-commits
mehdi_amini added a comment. Also I don't think the same option should be used for the parallel LTO codegen: it actually does not generate the same binary, which should deserve a dedicated opt-in (What if I mix ThinLTO and LTO, and I don't want // codegen?) https://reviews.llvm.org/D24826 _

Re: [PATCH] D24826: [LTO] Add -flto-jobs=N to control backend parallelism

2016-09-22 Thread Mehdi AMINI via cfe-commits
mehdi_amini added a comment. The Gold path looks fine. On OSX, we would have the clang driver relying on a LLVM cl::opt, for which I don't think there is any precedent. CC Duncan for advice. https://reviews.llvm.org/D24826 ___ cfe-commits mailing