[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

2018-11-03 Thread Petr Hosek via Phabricator via cfe-commits
phosek closed this revision.
phosek added inline comments.



Comment at: lib/Driver/ToolChains/Fuchsia.cpp:128
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);

mcgrathr wrote:
> mcgrathr wrote:
> > MaskRay wrote:
> > > If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) 
> > > in that `-static` switches the whole link to its special static mode. (as 
> > > usually while you link libstdc++/libc++ statically, you can still link 
> > > other libraries normally)
> > > 
> > > In ld.bfd/lld, `-Bstatic` is synonym with `-static`.
> > I think this part of the change was unintentional and should be undone.
> > Using `--pop-state` obviates the need for `-Bdynamic` to undo `-Bstatic`, 
> > but `-Bstatic` is still the right switch here.
> Actually, it's wrong two ways: the `--pop-state` should come before `-lm`.  
> Neither `-static` nor `-Bstatic` should apply to `-lm` (or to `-lc` that 
> comes later, which `-static` might).  `-static` vs `-Bstatic` is only a 
> latent bug given lld, but the `-lm` issue breaks the Zircon build today.
> 
Addressed in D54082.


Repository:
  rC Clang

https://reviews.llvm.org/D53854



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

2018-11-03 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr reopened this revision.
mcgrathr added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Driver/ToolChains/Fuchsia.cpp:128
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);

mcgrathr wrote:
> MaskRay wrote:
> > If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) in 
> > that `-static` switches the whole link to its special static mode. (as 
> > usually while you link libstdc++/libc++ statically, you can still link 
> > other libraries normally)
> > 
> > In ld.bfd/lld, `-Bstatic` is synonym with `-static`.
> I think this part of the change was unintentional and should be undone.
> Using `--pop-state` obviates the need for `-Bdynamic` to undo `-Bstatic`, but 
> `-Bstatic` is still the right switch here.
Actually, it's wrong two ways: the `--pop-state` should come before `-lm`.  
Neither `-static` nor `-Bstatic` should apply to `-lm` (or to `-lc` that comes 
later, which `-static` might).  `-static` vs `-Bstatic` is only a latent bug 
given lld, but the `-lm` issue breaks the Zircon build today.



Repository:
  rC Clang

https://reviews.llvm.org/D53854



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

2018-11-03 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments.



Comment at: lib/Driver/ToolChains/Fuchsia.cpp:128
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);

MaskRay wrote:
> If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) in 
> that `-static` switches the whole link to its special static mode. (as 
> usually while you link libstdc++/libc++ statically, you can still link other 
> libraries normally)
> 
> In ld.bfd/lld, `-Bstatic` is synonym with `-static`.
I think this part of the change was unintentional and should be undone.
Using `--pop-state` obviates the need for `-Bdynamic` to undo `-Bstatic`, but 
`-Bstatic` is still the right switch here.


Repository:
  rC Clang

https://reviews.llvm.org/D53854



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

2018-11-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: lib/Driver/ToolChains/Fuchsia.cpp:128
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);

If Fuchsia doesn't use gold, it is fine. gold diverges from ld.bfd (lld) in 
that `-static` switches the whole link to its special static mode. (as usually 
while you link libstdc++/libc++ statically, you can still link other libraries 
normally)

In ld.bfd/lld, `-Bstatic` is synonym with `-static`.


Repository:
  rC Clang

https://reviews.llvm.org/D53854



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

2018-11-02 Thread Petr Hosek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346064: [Driver] Use -push-/-pop-state and -as-needed for 
libc++ on Fuchsia (authored by phosek, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53854?vs=171948&id=172478#toc

Repository:
  rC Clang

https://reviews.llvm.org/D53854

Files:
  lib/Driver/ToolChains/Fuchsia.cpp
  test/Driver/fuchsia.cpp


Index: lib/Driver/ToolChains/Fuchsia.cpp
===
--- lib/Driver/ToolChains/Fuchsia.cpp
+++ lib/Driver/ToolChains/Fuchsia.cpp
@@ -122,13 +122,14 @@
   if (ToolChain.ShouldLinkCXXStdlib(Args)) {
 bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) 
&&
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("--push-state");
+CmdArgs.push_back("--as-needed");
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bdynamic");
+CmdArgs.push_back("-lm");
+CmdArgs.push_back("--pop-state");
   }
-  CmdArgs.push_back("-lm");
 }
 
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
Index: test/Driver/fuchsia.cpp
===
--- test/Driver/fuchsia.cpp
+++ test/Driver/fuchsia.cpp
@@ -15,7 +15,11 @@
 // CHECK-NOT: crti.o
 // CHECK-NOT: crtbegin.o
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
-// CHECK: "-lc++" "-lm"
+// CHECK: "--push-state"
+// CHECK: "--as-needed"
+// CHECK: "-lc++"
+// CHECK: "-lm"
+// CHECK: "--pop-state"
 // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
@@ -29,8 +33,10 @@
 // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \
 // RUN: -fuse-ld=lld 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-STATIC
-// CHECK-STATIC: "-Bstatic"
+// CHECK-STATIC: "--push-state"
+// CHECK-STATIC: "--as-needed"
+// CHECK-STATIC: "-static"
 // CHECK-STATIC: "-lc++"
-// CHECK-STATIC: "-Bdynamic"
 // CHECK-STATIC: "-lm"
+// CHECK-STATIC: "--pop-state"
 // CHECK-STATIC: "-lc"


Index: lib/Driver/ToolChains/Fuchsia.cpp
===
--- lib/Driver/ToolChains/Fuchsia.cpp
+++ lib/Driver/ToolChains/Fuchsia.cpp
@@ -122,13 +122,14 @@
   if (ToolChain.ShouldLinkCXXStdlib(Args)) {
 bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) &&
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("--push-state");
+CmdArgs.push_back("--as-needed");
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bdynamic");
+CmdArgs.push_back("-lm");
+CmdArgs.push_back("--pop-state");
   }
-  CmdArgs.push_back("-lm");
 }
 
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
Index: test/Driver/fuchsia.cpp
===
--- test/Driver/fuchsia.cpp
+++ test/Driver/fuchsia.cpp
@@ -15,7 +15,11 @@
 // CHECK-NOT: crti.o
 // CHECK-NOT: crtbegin.o
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
-// CHECK: "-lc++" "-lm"
+// CHECK: "--push-state"
+// CHECK: "--as-needed"
+// CHECK: "-lc++"
+// CHECK: "-lm"
+// CHECK: "--pop-state"
 // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
@@ -29,8 +33,10 @@
 // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \
 // RUN: -fuse-ld=lld 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-STATIC
-// CHECK-STATIC: "-Bstatic"
+// CHECK-STATIC: "--push-state"
+// CHECK-STATIC: "--as-needed"
+// CHECK-STATIC: "-static"
 // CHECK-STATIC: "-lc++"
-// CHECK-STATIC: "-Bdynamic"
 // CHECK-STATIC: "-lm"
+// CHECK-STATIC: "--pop-state"
 // CHECK-STATIC: "-lc"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

2018-10-31 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 171948.
phosek marked 2 inline comments as done.

Repository:
  rC Clang

https://reviews.llvm.org/D53854

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.cpp


Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -15,7 +15,11 @@
 // CHECK-NOT: crti.o
 // CHECK-NOT: crtbegin.o
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
-// CHECK: "-lc++" "-lm"
+// CHECK: "--push-state"
+// CHECK: "--as-needed"
+// CHECK: "-lc++"
+// CHECK: "-lm"
+// CHECK: "--pop-state"
 // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
@@ -29,8 +33,10 @@
 // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \
 // RUN: -fuse-ld=lld 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-STATIC
-// CHECK-STATIC: "-Bstatic"
+// CHECK-STATIC: "--push-state"
+// CHECK-STATIC: "--as-needed"
+// CHECK-STATIC: "-static"
 // CHECK-STATIC: "-lc++"
-// CHECK-STATIC: "-Bdynamic"
 // CHECK-STATIC: "-lm"
+// CHECK-STATIC: "--pop-state"
 // CHECK-STATIC: "-lc"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -122,13 +122,14 @@
   if (ToolChain.ShouldLinkCXXStdlib(Args)) {
 bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) 
&&
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("--push-state");
+CmdArgs.push_back("--as-needed");
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bdynamic");
+CmdArgs.push_back("-lm");
+CmdArgs.push_back("--pop-state");
   }
-  CmdArgs.push_back("-lm");
 }
 
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);


Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -15,7 +15,11 @@
 // CHECK-NOT: crti.o
 // CHECK-NOT: crtbegin.o
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
-// CHECK: "-lc++" "-lm"
+// CHECK: "--push-state"
+// CHECK: "--as-needed"
+// CHECK: "-lc++"
+// CHECK: "-lm"
+// CHECK: "--pop-state"
 // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
@@ -29,8 +33,10 @@
 // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \
 // RUN: -fuse-ld=lld 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-STATIC
-// CHECK-STATIC: "-Bstatic"
+// CHECK-STATIC: "--push-state"
+// CHECK-STATIC: "--as-needed"
+// CHECK-STATIC: "-static"
 // CHECK-STATIC: "-lc++"
-// CHECK-STATIC: "-Bdynamic"
 // CHECK-STATIC: "-lm"
+// CHECK-STATIC: "--pop-state"
 // CHECK-STATIC: "-lc"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -122,13 +122,14 @@
   if (ToolChain.ShouldLinkCXXStdlib(Args)) {
 bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) &&
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("--push-state");
+CmdArgs.push_back("--as-needed");
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bdynamic");
+CmdArgs.push_back("-lm");
+CmdArgs.push_back("--pop-state");
   }
-  CmdArgs.push_back("-lm");
 }
 
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

2018-10-30 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision.
mcgrathr added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:125
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("-push-state");
+CmdArgs.push_back("-as-needed");

I'd use the `--` version of all these GNU-compatible options since that's the 
GNU-compatible syntax.
(But `-static` is still single-dash.)



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:132
   }
   CmdArgs.push_back("-lm");
 }

`-lm` belongs inside the `--as-needed` umbrella too.


Repository:
  rC Clang

https://reviews.llvm.org/D53854



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53854: [Driver] Use -push-/-pop-state and -as-needed for libc++ on Fuchsia

2018-10-29 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added a reviewer: mcgrathr.
Herald added a reviewer: EricWF.
Herald added a subscriber: cfe-commits.

This avoids introducing unnecessary DT_NEEDED entries when using
C++ driver for linking C code or C++ code that doesn't use C++
standard library.


Repository:
  rC Clang

https://reviews.llvm.org/D53854

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.cpp


Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -15,7 +15,11 @@
 // CHECK-NOT: crti.o
 // CHECK-NOT: crtbegin.o
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
-// CHECK: "-lc++" "-lm"
+// CHECK: "-push-state"
+// CHECK: "-as-needed"
+// CHECK: "-lc++"
+// CHECK: "-pop-state"
+// CHECK: "-lm"
 // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
@@ -29,8 +33,10 @@
 // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \
 // RUN: -fuse-ld=lld 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-STATIC
-// CHECK-STATIC: "-Bstatic"
+// CHECK-STATIC: "-push-state"
+// CHECK-STATIC: "-as-needed"
+// CHECK-STATIC: "-static"
 // CHECK-STATIC: "-lc++"
-// CHECK-STATIC: "-Bdynamic"
+// CHECK-STATIC: "-pop-state"
 // CHECK-STATIC: "-lm"
 // CHECK-STATIC: "-lc"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -122,11 +122,12 @@
   if (ToolChain.ShouldLinkCXXStdlib(Args)) {
 bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) 
&&
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("-push-state");
+CmdArgs.push_back("-as-needed");
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bdynamic");
+CmdArgs.push_back("-pop-state");
   }
   CmdArgs.push_back("-lm");
 }


Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -15,7 +15,11 @@
 // CHECK-NOT: crti.o
 // CHECK-NOT: crtbegin.o
 // CHECK: "-L[[SYSROOT]]{{/|}}lib"
-// CHECK: "-lc++" "-lm"
+// CHECK: "-push-state"
+// CHECK: "-as-needed"
+// CHECK: "-lc++"
+// CHECK: "-pop-state"
+// CHECK: "-lm"
 // CHECK: "{{.*[/\\]}}libclang_rt.builtins-x86_64.a"
 // CHECK: "-lc"
 // CHECK-NOT: crtend.o
@@ -29,8 +33,10 @@
 // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++ \
 // RUN: -fuse-ld=lld 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-STATIC
-// CHECK-STATIC: "-Bstatic"
+// CHECK-STATIC: "-push-state"
+// CHECK-STATIC: "-as-needed"
+// CHECK-STATIC: "-static"
 // CHECK-STATIC: "-lc++"
-// CHECK-STATIC: "-Bdynamic"
+// CHECK-STATIC: "-pop-state"
 // CHECK-STATIC: "-lm"
 // CHECK-STATIC: "-lc"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -122,11 +122,12 @@
   if (ToolChain.ShouldLinkCXXStdlib(Args)) {
 bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) &&
!Args.hasArg(options::OPT_static);
+CmdArgs.push_back("-push-state");
+CmdArgs.push_back("-as-needed");
 if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  CmdArgs.push_back("-static");
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bdynamic");
+CmdArgs.push_back("-pop-state");
   }
   CmdArgs.push_back("-lm");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits