[cfe-commits] [PATCH] Bitrig - multiple changes
David Hill
dhill at mindcry.org
Thu Oct 4 07:06:57 PDT 2012
Ping.
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