[PATCH] D61931: [Driver] Use --android-tls for Android ARM/AArch64 when lld is used

2019-05-17 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay abandoned this revision.
MaskRay added a comment.

Abandon in favor of D62055 


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61931/new/

https://reviews.llvm.org/D61931



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

[PATCH] D61931: [Driver] Use --android-tls for Android ARM/AArch64 when lld is used

2019-05-17 Thread Stephen Hines via Phabricator via cfe-commits 
srhines added a comment.

This LGTM if Ryan is happy with it. Thanks for taking care of getting a 
workaround implemented until this can be fixed.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61931/new/

https://reviews.llvm.org/D61931



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

[PATCH] D61931: [Driver] Use --android-tls for Android ARM/AArch64 when lld is used

2019-05-15 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay updated this revision to Diff 199574.
MaskRay added a comment.

Use Args.MakeArgString(ToolChain.GetLinkerPath());


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61931/new/

https://reviews.llvm.org/D61931

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/android-aarch64-link.cpp
  test/Driver/android-arm-link.cpp


Index: test/Driver/android-arm-link.cpp
===
--- /dev/null
+++ test/Driver/android-arm-link.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang -target arm-linux-androideabi -### %s 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=--android-tls /dev/null
+// RUN: %clang -target arm-linux-androideabi -fuse-ld=lld -### %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=TLS %s
+
+// TLS: --android-tls
Index: test/Driver/android-aarch64-link.cpp
===
--- test/Driver/android-aarch64-link.cpp
+++ test/Driver/android-aarch64-link.cpp
@@ -16,7 +16,13 @@
 // RUN:   -### -v %s 2> %t
 // RUN: FileCheck -check-prefix=MAX-PAGE-SIZE < %t %s
 //
+// RUN: %clang -target aarch64-linux-android -### %s 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=--android-tls /dev/null
+// RUN: %clang -target aarch64-linux-android -fuse-ld=lld -### %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=TLS %s
+//
 // GENERIC-ARM: --fix-cortex-a53-843419
 // CORTEX-A53: --fix-cortex-a53-843419
 // CORTEX-A57-NOT: --fix-cortex-a53-843419
 // MAX-PAGE-SIZE: max-page-size=4096
+// TLS: --android-tls
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -390,10 +390,22 @@
   CmdArgs.push_back("--fix-cortex-a53-843419");
   }
 
-  // Android does not allow shared text relocations. Emit a warning if the
-  // user's code contains any.
-  if (isAndroid)
-  CmdArgs.push_back("--warn-shared-textrel");
+  if (isAndroid) {
+// Android does not allow shared text relocations. Emit a warning if the
+// user's code contains any.
+CmdArgs.push_back("--warn-shared-textrel");
+
+// FIXME In lld, --android-tls is a temporary option that makes its TLS
+// layout compatible with Android Bionic on ARM/AArch64. Delete once the
+// reservation of extra TLS slots is done in a proper layer (e.g.
+// crtbegin_{dynamic,static}.o).
+if (Triple.isARM() || Triple.isAArch64()) {
+  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
+  if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
+  llvm::sys::path::stem(Exec).equals_lower("ld.lld"))
+CmdArgs.push_back("--android-tls");
+}
+  }
 
   for (const auto  : ToolChain.ExtraOpts)
 CmdArgs.push_back(Opt.c_str());


Index: test/Driver/android-arm-link.cpp
===
--- /dev/null
+++ test/Driver/android-arm-link.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang -target arm-linux-androideabi -### %s 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=--android-tls /dev/null
+// RUN: %clang -target arm-linux-androideabi -fuse-ld=lld -### %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=TLS %s
+
+// TLS: --android-tls
Index: test/Driver/android-aarch64-link.cpp
===
--- test/Driver/android-aarch64-link.cpp
+++ test/Driver/android-aarch64-link.cpp
@@ -16,7 +16,13 @@
 // RUN:   -### -v %s 2> %t
 // RUN: FileCheck -check-prefix=MAX-PAGE-SIZE < %t %s
 //
+// RUN: %clang -target aarch64-linux-android -### %s 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=--android-tls /dev/null
+// RUN: %clang -target aarch64-linux-android -fuse-ld=lld -### %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=TLS %s
+//
 // GENERIC-ARM: --fix-cortex-a53-843419
 // CORTEX-A53: --fix-cortex-a53-843419
 // CORTEX-A57-NOT: --fix-cortex-a53-843419
 // MAX-PAGE-SIZE: max-page-size=4096
+// TLS: --android-tls
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -390,10 +390,22 @@
   CmdArgs.push_back("--fix-cortex-a53-843419");
   }
 
-  // Android does not allow shared text relocations. Emit a warning if the
-  // user's code contains any.
-  if (isAndroid)
-  CmdArgs.push_back("--warn-shared-textrel");
+  if (isAndroid) {
+// Android does not allow shared text relocations. Emit a warning if the
+// user's code contains any.
+CmdArgs.push_back("--warn-shared-textrel");
+
+// FIXME In lld, --android-tls is a temporary option that makes its TLS
+// layout compatible with Android Bionic on ARM/AArch64. Delete once the
+// reservation of extra TLS slots is done in a proper layer (e.g.
+// crtbegin_{dynamic,static}.o).
+if (Triple.isARM() || Triple.isAArch64()) {
+  const char *Exec =

[PATCH] D61931: [Driver] Use --android-tls for Android ARM/AArch64 when lld is used

2019-05-15 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added inline comments.



Comment at: lib/Driver/ToolChains/Gnu.cpp:404
+  const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ);
+  if (A && StringRef(A->getValue()).contains("lld"))
+CmdArgs.push_back("--android-tls");

The logic used for Fuschia is more precise:

  const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
  if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
  llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {
CmdArgs.push_back("-z");
CmdArgs.push_back("rodynamic");
  }



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61931/new/

https://reviews.llvm.org/D61931



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

[PATCH] D61931: [Driver] Use --android-tls for Android ARM/AArch64 when lld is used

2019-05-15 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay created this revision.
MaskRay added reviewers: danalbert, pirama, rprichard, srhines.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.
Herald added a project: clang.

See D61825  for the temporary lld option 
--android-tls.


Repository:
  rC Clang

https://reviews.llvm.org/D61931

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/android-aarch64-link.cpp
  test/Driver/android-arm-link.cpp


Index: test/Driver/android-arm-link.cpp
===
--- /dev/null
+++ test/Driver/android-arm-link.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang -target arm-linux-androideabi -### %s 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=--android-tls /dev/null
+// RUN: %clang -target arm-linux-androideabi -fuse-ld=lld -### %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=TLS %s
+
+// TLS: --android-tls
Index: test/Driver/android-aarch64-link.cpp
===
--- test/Driver/android-aarch64-link.cpp
+++ test/Driver/android-aarch64-link.cpp
@@ -16,7 +16,13 @@
 // RUN:   -### -v %s 2> %t
 // RUN: FileCheck -check-prefix=MAX-PAGE-SIZE < %t %s
 //
+// RUN: %clang -target aarch64-linux-android -### %s 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=--android-tls /dev/null
+// RUN: %clang -target aarch64-linux-android -fuse-ld=lld -### %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=TLS %s
+//
 // GENERIC-ARM: --fix-cortex-a53-843419
 // CORTEX-A53: --fix-cortex-a53-843419
 // CORTEX-A57-NOT: --fix-cortex-a53-843419
 // MAX-PAGE-SIZE: max-page-size=4096
+// TLS: --android-tls
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -390,10 +390,21 @@
   CmdArgs.push_back("--fix-cortex-a53-843419");
   }
 
-  // Android does not allow shared text relocations. Emit a warning if the
-  // user's code contains any.
-  if (isAndroid)
-  CmdArgs.push_back("--warn-shared-textrel");
+  if (isAndroid) {
+// Android does not allow shared text relocations. Emit a warning if the
+// user's code contains any.
+CmdArgs.push_back("--warn-shared-textrel");
+
+// FIXME In lld, --android-tls is a temporary option that makes its TLS
+// layout compatible with Android Bionic on ARM/AArch64. Delete once the
+// reservation of extra TLS slots is done in a proper layer (e.g.
+// crtbegin_{dynamic,static}.o).
+if (Triple.isARM() || Triple.isAArch64()) {
+  const Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ);
+  if (A && StringRef(A->getValue()).contains("lld"))
+CmdArgs.push_back("--android-tls");
+}
+  }
 
   for (const auto  : ToolChain.ExtraOpts)
 CmdArgs.push_back(Opt.c_str());


Index: test/Driver/android-arm-link.cpp
===
--- /dev/null
+++ test/Driver/android-arm-link.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang -target arm-linux-androideabi -### %s 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=--android-tls /dev/null
+// RUN: %clang -target arm-linux-androideabi -fuse-ld=lld -### %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=TLS %s
+
+// TLS: --android-tls
Index: test/Driver/android-aarch64-link.cpp
===
--- test/Driver/android-aarch64-link.cpp
+++ test/Driver/android-aarch64-link.cpp
@@ -16,7 +16,13 @@
 // RUN:   -### -v %s 2> %t
 // RUN: FileCheck -check-prefix=MAX-PAGE-SIZE < %t %s
 //
+// RUN: %clang -target aarch64-linux-android -### %s 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=--android-tls /dev/null
+// RUN: %clang -target aarch64-linux-android -fuse-ld=lld -### %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=TLS %s
+//
 // GENERIC-ARM: --fix-cortex-a53-843419
 // CORTEX-A53: --fix-cortex-a53-843419
 // CORTEX-A57-NOT: --fix-cortex-a53-843419
 // MAX-PAGE-SIZE: max-page-size=4096
+// TLS: --android-tls
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -390,10 +390,21 @@
   CmdArgs.push_back("--fix-cortex-a53-843419");
   }
 
-  // Android does not allow shared text relocations. Emit a warning if the
-  // user's code contains any.
-  if (isAndroid)
-  CmdArgs.push_back("--warn-shared-textrel");
+  if (isAndroid) {
+// Android does not allow shared text relocations. Emit a warning if the
+// user's code contains any.
+CmdArgs.push_back("--warn-shared-textrel");
+
+// FIXME In lld, --android-tls is a temporary option that makes its TLS
+// layout compatible with Android Bionic on ARM/AArch64. Delete once the
+// reservation of extra TLS slots is done in a proper layer (e.g.
+// crtbegin_{dynamic,static}.o).
+if (Triple.isARM() || Triple.isAArch64()) {
+  const Arg *A =