[cfe-commits] [PATCH] Bitrig - multiple changes
Nico Weber
thakis at chromium.org
Thu Oct 4 23:43:39 PDT 2012
Hi Chandler,
if you care enough about bitrig to reject mostly harmless patches, it
kinda feels like you should also care enough to not ignore the
follow-up patch for over two days. This really discourages sending
small, focused patches.
Nico
On Fri, Oct 5, 2012 at 7:18 AM, David Hill <dhill at mindcry.org> wrote:
> OK?
>
> On Wed, Oct 03, 2012 at 12:23:29PM -0400, David Hill wrote:
>>OK, here is patch 1 (attached).
>>
>>Make Bitrig's clang understand -stdlib= correctly.
>>With this patch, Bitrig can use a different c++ library without pain and
>>within the normal command line paramenters.
>>
>>Please review.
>>
>>Thanks,
>>David Hill
>>
>>On Tue, Oct 02, 2012 at 11:42:35PM -0700, Chandler Carruth wrote:
>>> On Tue, Oct 2, 2012 at 10:29 PM, Nico Weber <thakis at chromium.org> wrote:
>>>
>>> > It looks like this only touches bitrig-related code, which was added
>>> > by you originally. So lgtm.
>>> >
>>>
>>> It doesn't look quite that good to me. ;] The reason I've not looked at
>>> this is in part because the patch is really hard to review:
>>>
>>>
>>> > >>>>The attached patch includes the following:
>>> > >>>>
>>> > >>>>- reorder linking of the libraries
>>> > >>>>- support linking with pthread_p when -pg is used.
>>> > >>>>- understand -stdlib= correctly
>>> > >>>>- support --sysroot
>>> > >>>>- add tests for LD and -pg
>>> >
>>>
>>> Can you break this up into 4 or 5 patches, making them more targeted? It's
>>> hard to tell what is and isn't tested here, or what changes have to do with
>>> each other.
>>>
>>> Also, explain in more detail (maybe even in comments) for some of these --
>>> 'reorder linking' isn't terribly clear what the end goal is or what was
>>> necessary to get there.
>>>
>>> >>>>
>>> > >>>>Please review. Thank you!
>>> > >>>>
>>> > >>>>David Hill
>>> > >>>
>>> > >>>>diff --git a/lib/Driver/ToolChains.cpp b/lib/Driver/ToolChains.cpp
>>> > >>>>index 7e19551..4ce8dd3 100644
>>> > >>>>--- a/lib/Driver/ToolChains.cpp
>>> > >>>>+++ b/lib/Driver/ToolChains.cpp
>>> > >>>>@@ -1625,19 +1625,42 @@ void
>>> > Bitrig::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
>>> > >>>> DriverArgs.hasArg(options::OPT_nostdincxx))
>>> > >>>> return;
>>> > >>>>
>>> > >>>>- std::string Triple = getTriple().str();
>>> > >>>>- if (Triple.substr(0, 5) == "amd64")
>>> > >>>>- Triple.replace(0, 5, "x86_64");
>>> > >>>>-
>>> > >>>>- addSystemInclude(DriverArgs, CC1Args, "/usr/include/c++/4.6.2");
>>> > >>>>- addSystemInclude(DriverArgs, CC1Args,
>>> > "/usr/include/c++/4.6.2/backward");
>>> > >>>>- addSystemInclude(DriverArgs, CC1Args, "/usr/include/c++/4.6.2/" +
>>> > Triple);
>>> > >>>>+ CXXStdlibType Type = GetCXXStdlibType(DriverArgs);
>>> > >>>>+ switch (Type) {
>>> > >>>>+ case ToolChain::CST_Libcxx:
>>> > >>>>+ addSystemInclude(DriverArgs, CC1Args,
>>> > >>>>+ getDriver().SysRoot + "/usr/include/c++/");
>>> > >>>>+ break;
>>> > >>>>+ case ToolChain::CST_Libstdcxx:
>>> > >>>>+ std::string Triple = getTriple().str();
>>> > >>>>+ if (Triple.substr(0, 5) == "amd64")
>>> > >>>>+ Triple.replace(0, 5, "x86_64");
>>> > >>>>
>>> > >>>>+ addSystemInclude(DriverArgs, CC1Args,
>>> > >>>>+ getDriver().SysRoot + "/usr/include/c++/stdc++");
>>> > >>>>+ addSystemInclude(DriverArgs, CC1Args,
>>> > >>>>+ getDriver().SysRoot +
>>> > "/usr/include/c++/stdc++/backward");
>>> > >>>>+ addSystemInclude(DriverArgs, CC1Args,
>>> > >>>>+ getDriver().SysRoot + "/usr/include/c++/stdc++/"
>>> > + Triple);
>>> > >>>>+ break;
>>> > >>>>+ }
>>> > >>>> }
>>> > >>>>
>>> > >>>> void Bitrig::AddCXXStdlibLibArgs(const ArgList &Args,
>>> > >>>> ArgStringList &CmdArgs) const {
>>> > >>>>- CmdArgs.push_back("-lstdc++");
>>> > >>>>+ CXXStdlibType Type = GetCXXStdlibType(Args);
>>> > >>>>+
>>> > >>>>+ switch (Type) {
>>> > >>>>+ case ToolChain::CST_Libcxx:
>>> > >>>>+ CmdArgs.push_back("-lc++");
>>> > >>>>+ CmdArgs.push_back("-lcxxrt");
>>> > >>>>+ /* for now we borrow Unwind from supc++ */
>>> > >>>>+ CmdArgs.push_back("-lgcc");
>>> > >>>>+ break;
>>> > >>>>+ case ToolChain::CST_Libstdcxx:
>>> > >>>>+ CmdArgs.push_back("-lstdc++");
>>> > >>>>+ break;
>>> > >>>>+ }
>>> > >>>> }
>>> > >>>>
>>> > >>>> /// FreeBSD - FreeBSD tool chain which can call as(1) and ld(1)
>>> > directly.
>>> > >>>>diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp
>>> > >>>>index 0866c01..9aea69b 100644
>>> > >>>>--- a/lib/Driver/Tools.cpp
>>> > >>>>+++ b/lib/Driver/Tools.cpp
>>> > >>>>@@ -5086,23 +5086,6 @@ void bitrig::Link::ConstructJob(Compilation &C,
>>> > const JobAction &JA,
>>> > >>>>
>>> > >>>> if (!Args.hasArg(options::OPT_nostdlib) &&
>>> > >>>> !Args.hasArg(options::OPT_nodefaultlibs)) {
>>> > >>>>- if (D.CCCIsCXX) {
>>> > >>>>- getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
>>> > >>>>- if (Args.hasArg(options::OPT_pg))
>>> > >>>>- CmdArgs.push_back("-lm_p");
>>> > >>>>- else
>>> > >>>>- CmdArgs.push_back("-lm");
>>> > >>>>- }
>>> > >>>>-
>>> > >>>>- if (Args.hasArg(options::OPT_pthread))
>>> > >>>>- CmdArgs.push_back("-lpthread");
>>> > >>>>- if (!Args.hasArg(options::OPT_shared)) {
>>> > >>>>- if (Args.hasArg(options::OPT_pg))
>>> > >>>>- CmdArgs.push_back("-lc_p");
>>> > >>>>- else
>>> > >>>>- CmdArgs.push_back("-lc");
>>> > >>>>- }
>>> > >>>>-
>>> > >>>> std::string myarch = "-lclang_rt.";
>>> > >>>> const llvm::Triple &T = getToolChain().getTriple();
>>> > >>>> llvm::Triple::ArchType Arch = T.getArch();
>>> > >>>>@@ -5119,7 +5102,31 @@ void bitrig::Link::ConstructJob(Compilation &C,
>>> > const JobAction &JA,
>>> > >>>> default:
>>> > >>>> assert(0 && "Unsupported architecture");
>>> > >>>> }
>>> > >>>>+
>>> > >>>> CmdArgs.push_back(Args.MakeArgString(myarch));
>>> > >>>>+
>>> > >>>>+ if (D.CCCIsCXX) {
>>> > >>>>+ getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
>>> > >>>>+ if (Args.hasArg(options::OPT_pg))
>>> > >>>>+ CmdArgs.push_back("-lm_p");
>>> > >>>>+ else
>>> > >>>>+ CmdArgs.push_back("-lm");
>>> > >>>>+ }
>>> > >>>>+
>>> > >>>>+ if (Args.hasArg(options::OPT_pthread)) {
>>> > >>>>+ if (!Args.hasArg(options::OPT_shared) &&
>>> > >>>>+ Args.hasArg(options::OPT_pg))
>>> > >>>>+ CmdArgs.push_back("-lpthread_p");
>>> > >>>>+ else
>>> > >>>>+ CmdArgs.push_back("-lpthread");
>>> > >>>>+ }
>>> > >>>>+
>>> > >>>>+ if (!Args.hasArg(options::OPT_shared)) {
>>> > >>>>+ if (Args.hasArg(options::OPT_pg))
>>> > >>>>+ CmdArgs.push_back("-lc_p");
>>> > >>>>+ else
>>> > >>>>+ CmdArgs.push_back("-lc");
>>> > >>>>+ }
>>> > >>>> }
>>> > >>>>
>>> > >>>> if (!Args.hasArg(options::OPT_nostdlib) &&
>>> > >>>>diff --git a/test/Driver/bitrig.c b/test/Driver/bitrig.c
>>> > >>>>new file mode 100644
>>> > >>>>index 0000000..412e79c
>>> > >>>>--- /dev/null
>>> > >>>>+++ b/test/Driver/bitrig.c
>>> > >>>>@@ -0,0 +1,9 @@
>>> > >>>>+// RUN: %clang -no-canonical-prefixes -ccc-clang-archs "" -target
>>> > amd64-pc-bitrig %s -### 2>&1 \
>>> > >>>>+// RUN: | FileCheck --check-prefix=CHECK-LD %s
>>> > >>>>+// CHECK-LD: clang{{.*}}" "-cc1" "-triple" "amd64-pc-bitrig"
>>> > >>>>+// CHECK-LD: ld{{.*}}" "-e" "__start" "--eh-frame-hdr" "-Bdynamic"
>>> > "-dynamic-linker" "{{.*}}ld.so" "-o" "a.out" "{{.*}}crt0.o"
>>> > "{{.*}}crtbegin.o" "{{.*}}.o" "-lclang_rt.amd64" "-lc" "{{.*}}crtend.o"
>>> > >>>>+
>>> > >>>>+// RUN: %clang -no-canonical-prefixes -ccc-clang-archs "" -target
>>> > amd64-pc-bitrig -pg -pthread %s -### 2>&1 \
>>> > >>>>+// RUN: | FileCheck --check-prefix=CHECK-PG %s
>>> > >>>>+// CHECK-PG: clang{{.*}}" "-cc1" "-triple" "amd64-pc-bitrig"
>>> > >>>>+// CHECK-PG: ld{{.*}}" "-e" "__start" "--eh-frame-hdr" "-Bdynamic"
>>> > "-dynamic-linker" "{{.*}}ld.so" "-o" "a.out" "{{.*}}crt0.o"
>>> > "{{.*}}crtbegin.o" "{{.*}}.o" "-lclang_rt.amd64" "-lpthread_p" "-lc_p"
>>> > "{{.*}}crtend.o"
>>> > >>>
>>> > >>>>_______________________________________________
>>> > >>>>cfe-commits mailing list
>>> > >>>>cfe-commits at cs.uiuc.edu
>>> > >>>>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>> > >>>
>>> > >>>_______________________________________________
>>> > >>>cfe-commits mailing list
>>> > >>>cfe-commits at cs.uiuc.edu
>>> > >>>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>> > >>_______________________________________________
>>> > >>cfe-commits mailing list
>>> > >>cfe-commits at cs.uiuc.edu
>>> > >>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>> > > _______________________________________________
>>> > > cfe-commits mailing list
>>> > > cfe-commits at cs.uiuc.edu
>>> > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>> > _______________________________________________
>>> > cfe-commits mailing list
>>> > cfe-commits at cs.uiuc.edu
>>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>> >
>
>>diff --git a/lib/Driver/ToolChains.cpp b/lib/Driver/ToolChains.cpp
>>index 81f38878..2c841ff 100644
>>--- a/lib/Driver/ToolChains.cpp
>>+++ b/lib/Driver/ToolChains.cpp
>>@@ -1665,19 +1665,42 @@ void Bitrig::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
>> DriverArgs.hasArg(options::OPT_nostdincxx))
>> return;
>>
>>- std::string Triple = getTriple().str();
>>- if (Triple.substr(0, 5) == "amd64")
>>- Triple.replace(0, 5, "x86_64");
>>-
>>- addSystemInclude(DriverArgs, CC1Args, "/usr/include/c++/4.6.2");
>>- addSystemInclude(DriverArgs, CC1Args, "/usr/include/c++/4.6.2/backward");
>>- addSystemInclude(DriverArgs, CC1Args, "/usr/include/c++/4.6.2/" + Triple);
>>+ CXXStdlibType Type = GetCXXStdlibType(DriverArgs);
>>+ switch (Type) {
>>+ case ToolChain::CST_Libcxx:
>>+ addSystemInclude(DriverArgs, CC1Args,
>>+ getDriver().SysRoot + "/usr/include/c++/");
>>+ break;
>>+ case ToolChain::CST_Libstdcxx:
>>+ std::string Triple = getTriple().str();
>>+ if (Triple.substr(0, 5) == "amd64")
>>+ Triple.replace(0, 5, "x86_64");
>>
>>+ addSystemInclude(DriverArgs, CC1Args,
>>+ getDriver().SysRoot + "/usr/include/c++/stdc++");
>>+ addSystemInclude(DriverArgs, CC1Args,
>>+ getDriver().SysRoot + "/usr/include/c++/stdc++/backward");
>>+ addSystemInclude(DriverArgs, CC1Args,
>>+ getDriver().SysRoot + "/usr/include/c++/stdc++/" + Triple);
>>+ break;
>>+ }
>> }
>>
>> void Bitrig::AddCXXStdlibLibArgs(const ArgList &Args,
>> ArgStringList &CmdArgs) const {
>>- CmdArgs.push_back("-lstdc++");
>>+ CXXStdlibType Type = GetCXXStdlibType(Args);
>>+
>>+ switch (Type) {
>>+ case ToolChain::CST_Libcxx:
>>+ CmdArgs.push_back("-lc++");
>>+ CmdArgs.push_back("-lcxxrt");
>>+ /* for now we borrow Unwind from supc++ */
>>+ CmdArgs.push_back("-lgcc");
>>+ break;
>>+ case ToolChain::CST_Libstdcxx:
>>+ CmdArgs.push_back("-lstdc++");
>>+ break;
>>+ }
>> }
>
>>_______________________________________________
>>cfe-commits mailing list
>>cfe-commits at cs.uiuc.edu
>>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list