[cfe-commits] [PATCH] Bitrig - multiple changes
Chandler Carruth
chandlerc at google.com
Tue Oct 2 23:42:35 PDT 2012
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121002/c052ea22/attachment.html>
More information about the cfe-commits
mailing list