[PATCH] D38599: Remove warnings for dynamic_cast fallback.

2017-10-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

I assume std::type_info::operator== also needs to be adjusted to compare string 
names? It looks like libc++'s version of the function does string comparisons 
for ARM64 iOS, but only on some classes (e.g. public(?) ones). Look for the 
_LIBCPP_HAS_NONUNIQUE_TYPEINFO and _LIBCPP_NONUNIQUE_RTTI_BIT flags. I wonder 
if ARM64 iOS also sets _LIBCXX_DYNAMIC_FALLBACK for libc++abi.

I noticed that Clang on GNU/Linux is treating two classes defined in separate 
anonymous namespaces as equal for std::type_info::operator== and dynamic_cast, 
which I *think* is a Clang bug. G++ marks the typeinfo names with an asterisk 
to disable the string comparisons. I'd expect that enabling 
_LIBCXX_DYNAMIC_FALLBACK would introduce the same bug into the Android 
toolchain.


Repository:
  rL LLVM

https://reviews.llvm.org/D38599



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

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

2017-10-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

Here's the Clang bug I filed: https://bugs.llvm.org/show_bug.cgi?id=34907


Repository:
  rL LLVM

https://reviews.llvm.org/D38599



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

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

2017-10-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

Some relevant code links:

- 
https://github.com/llvm-mirror/clang/blob/8c9bf999aa40ab6077b958b5edcf587b9d76ce24/lib/CodeGen/ItaniumCXXABI.cpp#L359
 ==> iOS64CXXABI overrides shouldRTTIBeUnique to return false.
- 
https://github.com/llvm-mirror/libcxx/blob/ca79c159d8bfbe190a6cbfce74eb2d050697d8b9/include/typeinfo#L176
 ==> libc++ type_info::operator== uses string comparison, but only if 
_LIBCPP_NONUNIQUE_RTTI_BIT has been OR'ed into the __type_name pointers of both 
type_info objects
- 
https://github.com/llvm-mirror/libcxx/blob/ca79c159d8bfbe190a6cbfce74eb2d050697d8b9/include/__config#L879
 ==> use the highest bit for ARM64 iOS


Repository:
  rL LLVM

https://reviews.llvm.org/D38599



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

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

2017-10-17 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

In https://reviews.llvm.org/D38599#899198, @danalbert wrote:

> In https://reviews.llvm.org/D38599#899196, @howard.hinnant wrote:
>
> > Fwiw, I wrote this code.  All of that "fallback" stuff was written to make 
> > customer code that was incorrect, but working on OS X 
> > -version-that-used-libsupc++ continue to work.  I.e. to be a crutch for 
> > incorrect code.  It should all be removed if you no longer want to provide 
> > such a crutch.
>
>
> Android may end up wanting to use it for the same reason. I've backed it out 
> for now, but it may end up being something we need since a fair number of NDK 
> users (majority, probably) use libsupc++ and that might be a point of pain in 
> migrating to libc++abi. Other Linux distros may want it as well, so probably 
> worth leaving the code around.


On OS X, dynamic vague linkage appears to work by default, and 
_LIBCXX_DYNAMIC_FALLBACK catches some code that slips through the cracks (e.g. 
code that does something odd like: specify visibility attributes, explicitly 
specify RTLD_LOCAL, link with -Bsymbolic, etc.). dlopen defaults to 
RTLD_GLOBAL, which merges weak definitions, even with an executable. 
System.loadLibrary uses RTLD_GLOBAL.

On Android, AFAICT, dynamic vague linkage doesn't work prior to M, and with M, 
it only works between shared objects loaded in one batch. Loading multiple C++ 
shared objects one-at-a-time generally results in duplicated type_info objects, 
which break the various RTTI-dependent features when libc++/libc++abi are used.

_LIBCXX_DYNAMIC_FALLBACK doesn't generally fix __dynamic_cast to account for 
duplicated type_info objects. Instead, it flags a specific situation that's 
easy to detect. If we have code like this...

  Static *s = new Derived;
  dynamic_cast(s)

... __dynamic_cast will search the RTTI graph of `Derived` looking for `Static` 
and `Dest`. It expects to find at least one instance of `Static`. If it 
doesn't, there's obviously something wrong, because, assuming the C++ ABI has 
been followed, the `s` pointer is not really of type `Static`! This situation 
is cheap to detect (a couple extra comparisons). If `Static` //is// found, the 
fallback doesn't activate, but dynamic_cast can still fail, e.g.:

- If a `Dest` in the RTTI graph doesn't match `&typeid(Dest)` where 
dynamic_cast is used, then dynamic_cast can incorrectly return NULL.
- If there are two `Dest` objects in the RTTI graph with unequal addresses, 
then dynamic_cast can incorrectly return non-NULL when the cast should have 
been ambiguous. (Writing this test case is a fun little exercise.)
- Multiple `Static` objects with unequal addresses probably causes similar 
failures, as long as one of them matches `&typeid(Static)` where dynamic_cast 
is used.

I think _LIBCXX_DYNAMIC_FALLBACK is fine to enable on Android, but (1) it seems 
insufficient, (2) I'm skeptical about removing the diagnostic, and (3) a 
general solution to the libc++abi RTTI problem tends to make 
_LIBCXX_DYNAMIC_FALLBACK irrelevant. i.e. We may need to simply compare 
typeinfo strings like gnustl does (or the `#ifdef _WIN32` code for 
dynamic_cast), which subsumes _LIBCXX_DYNAMIC_FALLBACK.

I'm still intrigued by the `shouldRTTIBeUnique` / `_LIBCPP_NONUNIQUE_RTTI_BIT` 
stuff I linked above. I'm a little surprised that 64-bit iOS would need that 
behavior. Maybe we need to turn it on for Android, but where is the 
corresponding flag for libcxxabi?


https://reviews.llvm.org/D38599



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://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 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] D81622: [Clang] Search computed sysroot for libc++ header paths

2020-06-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard created this revision.
Herald added subscribers: cfe-commits, luismarques, apazos, sameer.abuasal, 
pzheng, s.egerton, lenary, Jim, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, MaskRay, jrtc27, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb.
Herald added a project: clang.
rprichard updated this revision to Diff 270005.
rprichard added a comment.

Rerun after installing clang-format


The Android NDK's clang driver is used with an Android -target setting,
and the driver automatically finds the Android sysroot at a path
relative to the driver. The sysroot has the libc++ headers in it.

Remove Hurd::computeSysRoot as it is equivalent to the new
ToolChain::computeSysRoot method.

Fixes PR46213.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81622

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.h
  clang/lib/Driver/ToolChains/MSP430.h
  clang/lib/Driver/ToolChains/RISCVToolchain.h

Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -39,7 +39,7 @@
   Tool *buildLinker() const override;
 
 private:
-  std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/MSP430.h
===
--- clang/lib/Driver/ToolChains/MSP430.h
+++ clang/lib/Driver/ToolChains/MSP430.h
@@ -44,7 +44,7 @@
   Tool *buildLinker() const override;
 
 private:
-  std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/Linux.h
===
--- clang/lib/Driver/ToolChains/Linux.h
+++ clang/lib/Driver/ToolChains/Linux.h
@@ -42,7 +42,7 @@
   SanitizerMask getSupportedSanitizers() const override;
   void addProfileRTLibs(const llvm::opt::ArgList &Args,
 llvm::opt::ArgStringList &CmdArgs) const override;
-  virtual std::string computeSysRoot() const;
+  virtual std::string computeSysRoot() const override;
 
   std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
Index: clang/lib/Driver/ToolChains/Hurd.h
===
--- clang/lib/Driver/ToolChains/Hurd.h
+++ clang/lib/Driver/ToolChains/Hurd.h
@@ -27,8 +27,6 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
-  std::string computeSysRoot() const;
-
   std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
   void addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const override;
Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -125,13 +125,6 @@
   return new tools::gnutools::Assembler(*this);
 }
 
-std::string Hurd::computeSysRoot() const {
-  if (!getDriver().SysRoot.empty())
-return getDriver().SysRoot;
-
-  return std::string();
-}
-
 std::string Hurd::getDynamicLinker(const ArgList &Args) const {
   if (getArch() == llvm::Triple::x86)
 return "/lib/ld.so";
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2846,7 +2846,6 @@
 void
 Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const {
-  const std::string& SysRoot = getDriver().SysRoot;
   auto AddIncludePath = [&](std::string Path) {
 std::string IncludePath = DetectLibcxxIncludePath(getVFS(), Path);
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
@@ -2862,6 +2861,7 @@
   // If this is a development, non-installed, clang, libcxx will
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
+  std::string SysRoot = computeSysRoot();
   if (AddIncludePath(SysRoot + "/usr/local/include/c++"))
 return;
   if (AddIncludePath(SysRoot + "/usr/include/c++"))
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -750,6 +750,8 @@
   return ComputeLLVMTriple(Args, InputType);
 }
 
+std::string ToolChain::computeSysRoot() const { return D.SysRoot; }
+
 void ToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

2020-06-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard updated this revision to Diff 270005.
rprichard added a comment.

Rerun after installing clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81622

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.h
  clang/lib/Driver/ToolChains/MSP430.h
  clang/lib/Driver/ToolChains/RISCVToolchain.h

Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -39,7 +39,7 @@
   Tool *buildLinker() const override;
 
 private:
-  std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/MSP430.h
===
--- clang/lib/Driver/ToolChains/MSP430.h
+++ clang/lib/Driver/ToolChains/MSP430.h
@@ -44,7 +44,7 @@
   Tool *buildLinker() const override;
 
 private:
-  std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/Linux.h
===
--- clang/lib/Driver/ToolChains/Linux.h
+++ clang/lib/Driver/ToolChains/Linux.h
@@ -42,7 +42,7 @@
   SanitizerMask getSupportedSanitizers() const override;
   void addProfileRTLibs(const llvm::opt::ArgList &Args,
 llvm::opt::ArgStringList &CmdArgs) const override;
-  virtual std::string computeSysRoot() const;
+  virtual std::string computeSysRoot() const override;
 
   std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
Index: clang/lib/Driver/ToolChains/Hurd.h
===
--- clang/lib/Driver/ToolChains/Hurd.h
+++ clang/lib/Driver/ToolChains/Hurd.h
@@ -27,8 +27,6 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
-  std::string computeSysRoot() const;
-
   std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
   void addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const override;
Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -125,13 +125,6 @@
   return new tools::gnutools::Assembler(*this);
 }
 
-std::string Hurd::computeSysRoot() const {
-  if (!getDriver().SysRoot.empty())
-return getDriver().SysRoot;
-
-  return std::string();
-}
-
 std::string Hurd::getDynamicLinker(const ArgList &Args) const {
   if (getArch() == llvm::Triple::x86)
 return "/lib/ld.so";
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2846,7 +2846,6 @@
 void
 Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const {
-  const std::string& SysRoot = getDriver().SysRoot;
   auto AddIncludePath = [&](std::string Path) {
 std::string IncludePath = DetectLibcxxIncludePath(getVFS(), Path);
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
@@ -2862,6 +2861,7 @@
   // If this is a development, non-installed, clang, libcxx will
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
+  std::string SysRoot = computeSysRoot();
   if (AddIncludePath(SysRoot + "/usr/local/include/c++"))
 return;
   if (AddIncludePath(SysRoot + "/usr/include/c++"))
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -750,6 +750,8 @@
   return ComputeLLVMTriple(Args, InputType);
 }
 
+std::string ToolChain::computeSysRoot() const { return D.SysRoot; }
+
 void ToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {
   // Each toolchain should provide the appropriate include flags.
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -536,6 +536,10 @@
   /// FIXME: this really belongs on some sort of DeploymentTarget abstraction
   virtual bool hasBlocksRuntime() const { return true; }
 
+  /// Return the sysroot, possibly searching for a default sysroot using
+  /// tar

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

2020-06-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard updated this revision to Diff 270009.
rprichard added a comment.

Reverse clang-format's change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81622

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.h
  clang/lib/Driver/ToolChains/MSP430.h
  clang/lib/Driver/ToolChains/RISCVToolchain.h

Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -39,7 +39,7 @@
   Tool *buildLinker() const override;
 
 private:
-  std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/MSP430.h
===
--- clang/lib/Driver/ToolChains/MSP430.h
+++ clang/lib/Driver/ToolChains/MSP430.h
@@ -44,7 +44,7 @@
   Tool *buildLinker() const override;
 
 private:
-  std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/Linux.h
===
--- clang/lib/Driver/ToolChains/Linux.h
+++ clang/lib/Driver/ToolChains/Linux.h
@@ -42,7 +42,7 @@
   SanitizerMask getSupportedSanitizers() const override;
   void addProfileRTLibs(const llvm::opt::ArgList &Args,
 llvm::opt::ArgStringList &CmdArgs) const override;
-  virtual std::string computeSysRoot() const;
+  virtual std::string computeSysRoot() const override;
 
   std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
Index: clang/lib/Driver/ToolChains/Hurd.h
===
--- clang/lib/Driver/ToolChains/Hurd.h
+++ clang/lib/Driver/ToolChains/Hurd.h
@@ -27,8 +27,6 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
-  std::string computeSysRoot() const;
-
   std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
   void addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const override;
Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -125,13 +125,6 @@
   return new tools::gnutools::Assembler(*this);
 }
 
-std::string Hurd::computeSysRoot() const {
-  if (!getDriver().SysRoot.empty())
-return getDriver().SysRoot;
-
-  return std::string();
-}
-
 std::string Hurd::getDynamicLinker(const ArgList &Args) const {
   if (getArch() == llvm::Triple::x86)
 return "/lib/ld.so";
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2846,7 +2846,6 @@
 void
 Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const {
-  const std::string& SysRoot = getDriver().SysRoot;
   auto AddIncludePath = [&](std::string Path) {
 std::string IncludePath = DetectLibcxxIncludePath(getVFS(), Path);
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
@@ -2862,6 +2861,7 @@
   // If this is a development, non-installed, clang, libcxx will
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
+  std::string SysRoot = computeSysRoot();
   if (AddIncludePath(SysRoot + "/usr/local/include/c++"))
 return;
   if (AddIncludePath(SysRoot + "/usr/include/c++"))
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -750,6 +750,10 @@
   return ComputeLLVMTriple(Args, InputType);
 }
 
+std::string ToolChain::computeSysRoot() const {
+  return D.SysRoot;
+}
+
 void ToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {
   // Each toolchain should provide the appropriate include flags.
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -536,6 +536,10 @@
   /// FIXME: this really belongs on some sort of DeploymentTarget abstraction
   virtual bool hasBlocksRuntime() const { return true; }
 
+  /// Return the sysroot, possibly searching for a default sysroot using
+  /// tar

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

2020-06-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added reviewers: srhines, danalbert, libc++, kristina.
rprichard added a comment.

FWIW, `computeSysRoot` is overridden by these toolchains:

- Linux
- MSP430ToolChain
- MipsLLVMToolChain
- RISCVToolChain

I think this change restores the pre D69758  
behavior for Linux and MipsLLVMToolChain. It might be nice to have a review 
from someone familiar with the MSP430 and RISC-V toolchains.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81622



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

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

2020-06-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard updated this revision to Diff 270030.
rprichard added a comment.

Remove unnecessary `virtual` keyword.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81622

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.h
  clang/lib/Driver/ToolChains/MSP430.h
  clang/lib/Driver/ToolChains/RISCVToolchain.h

Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -39,7 +39,7 @@
   Tool *buildLinker() const override;
 
 private:
-  std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/MSP430.h
===
--- clang/lib/Driver/ToolChains/MSP430.h
+++ clang/lib/Driver/ToolChains/MSP430.h
@@ -44,7 +44,7 @@
   Tool *buildLinker() const override;
 
 private:
-  std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/Linux.h
===
--- clang/lib/Driver/ToolChains/Linux.h
+++ clang/lib/Driver/ToolChains/Linux.h
@@ -42,7 +42,7 @@
   SanitizerMask getSupportedSanitizers() const override;
   void addProfileRTLibs(const llvm::opt::ArgList &Args,
 llvm::opt::ArgStringList &CmdArgs) const override;
-  virtual std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 
   std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
Index: clang/lib/Driver/ToolChains/Hurd.h
===
--- clang/lib/Driver/ToolChains/Hurd.h
+++ clang/lib/Driver/ToolChains/Hurd.h
@@ -27,8 +27,6 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
-  std::string computeSysRoot() const;
-
   std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
   void addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const override;
Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -125,13 +125,6 @@
   return new tools::gnutools::Assembler(*this);
 }
 
-std::string Hurd::computeSysRoot() const {
-  if (!getDriver().SysRoot.empty())
-return getDriver().SysRoot;
-
-  return std::string();
-}
-
 std::string Hurd::getDynamicLinker(const ArgList &Args) const {
   if (getArch() == llvm::Triple::x86)
 return "/lib/ld.so";
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2846,7 +2846,6 @@
 void
 Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const {
-  const std::string& SysRoot = getDriver().SysRoot;
   auto AddIncludePath = [&](std::string Path) {
 std::string IncludePath = DetectLibcxxIncludePath(getVFS(), Path);
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
@@ -2862,6 +2861,7 @@
   // If this is a development, non-installed, clang, libcxx will
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
+  std::string SysRoot = computeSysRoot();
   if (AddIncludePath(SysRoot + "/usr/local/include/c++"))
 return;
   if (AddIncludePath(SysRoot + "/usr/include/c++"))
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -750,6 +750,10 @@
   return ComputeLLVMTriple(Args, InputType);
 }
 
+std::string ToolChain::computeSysRoot() const {
+  return D.SysRoot;
+}
+
 void ToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {
   // Each toolchain should provide the appropriate include flags.
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -536,6 +536,10 @@
   /// FIXME: this really belongs on some sort of DeploymentTarget abstraction
   virtual bool hasBlocksRuntime() const { return true; }
 
+  /// Return the sysroot, possibly searching for a default sysroot using
+  /// targ

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

2020-06-17 Thread Ryan Prichard via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c4ce202267e: [Driver] Search computed sysroot for libc++ 
header paths (authored by rprichard).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81622

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.h
  clang/lib/Driver/ToolChains/MSP430.h
  clang/lib/Driver/ToolChains/RISCVToolchain.h

Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -39,7 +39,7 @@
   Tool *buildLinker() const override;
 
 private:
-  std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/MSP430.h
===
--- clang/lib/Driver/ToolChains/MSP430.h
+++ clang/lib/Driver/ToolChains/MSP430.h
@@ -44,7 +44,7 @@
   Tool *buildLinker() const override;
 
 private:
-  std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/Linux.h
===
--- clang/lib/Driver/ToolChains/Linux.h
+++ clang/lib/Driver/ToolChains/Linux.h
@@ -42,7 +42,7 @@
   SanitizerMask getSupportedSanitizers() const override;
   void addProfileRTLibs(const llvm::opt::ArgList &Args,
 llvm::opt::ArgStringList &CmdArgs) const override;
-  virtual std::string computeSysRoot() const;
+  std::string computeSysRoot() const override;
 
   std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
Index: clang/lib/Driver/ToolChains/Hurd.h
===
--- clang/lib/Driver/ToolChains/Hurd.h
+++ clang/lib/Driver/ToolChains/Hurd.h
@@ -27,8 +27,6 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
-  std::string computeSysRoot() const;
-
   std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
 
   void addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const override;
Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -125,13 +125,6 @@
   return new tools::gnutools::Assembler(*this);
 }
 
-std::string Hurd::computeSysRoot() const {
-  if (!getDriver().SysRoot.empty())
-return getDriver().SysRoot;
-
-  return std::string();
-}
-
 std::string Hurd::getDynamicLinker(const ArgList &Args) const {
   if (getArch() == llvm::Triple::x86)
 return "/lib/ld.so";
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2846,7 +2846,6 @@
 void
 Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const {
-  const std::string& SysRoot = getDriver().SysRoot;
   auto AddIncludePath = [&](std::string Path) {
 std::string IncludePath = DetectLibcxxIncludePath(getVFS(), Path);
 if (IncludePath.empty() || !getVFS().exists(IncludePath))
@@ -2862,6 +2861,7 @@
   // If this is a development, non-installed, clang, libcxx will
   // not be found at ../include/c++ but it likely to be found at
   // one of the following two locations:
+  std::string SysRoot = computeSysRoot();
   if (AddIncludePath(SysRoot + "/usr/local/include/c++"))
 return;
   if (AddIncludePath(SysRoot + "/usr/include/c++"))
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -750,6 +750,10 @@
   return ComputeLLVMTriple(Args, InputType);
 }
 
+std::string ToolChain::computeSysRoot() const {
+  return D.SysRoot;
+}
+
 void ToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {
   // Each toolchain should provide the appropriate include flags.
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -536,6 +536,10 @@
   /// FIXME: this really belongs on some sort of DeploymentTarget abstraction
   virtual bool hasBlocksRuntime() const { return true; }
 
+  /// Ret

[PATCH] D81622: [Clang] Search computed sysroot for libc++ header paths

2020-06-17 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

Ok, I'll run that command in the future, and I'll watch D80978 
 for updates.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81622



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

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

2022-07-20 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard abandoned this revision.
rprichard added a comment.

Abandoning this patch because D127465  is a 
better fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127267

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

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

2022-07-20 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard updated this revision to Diff 446302.
rprichard added a comment.

Stylistic change: keep the -ALIGN32 and -ALIGN64 suffixes in the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D28213

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/init-x86.c
  clang/test/Sema/atomic-ops.c


Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -33,11 +33,7 @@
 _Static_assert(__GCC_ATOMIC_INT_LOCK_FREE == __CLANG_ATOMIC_INT_LOCK_FREE, "");
 _Static_assert(__GCC_ATOMIC_LONG_LOCK_FREE == 2, "");
 _Static_assert(__GCC_ATOMIC_LONG_LOCK_FREE == __CLANG_ATOMIC_LONG_LOCK_FREE, 
"");
-#ifdef __i386__
-_Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == 1, "");
-#else
 _Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == 2, "");
-#endif
 _Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == __CLANG_ATOMIC_LLONG_LOCK_FREE, 
"");
 _Static_assert(__GCC_ATOMIC_POINTER_LOCK_FREE == 2, "");
 _Static_assert(__GCC_ATOMIC_POINTER_LOCK_FREE == 
__CLANG_ATOMIC_POINTER_LOCK_FREE, "");
Index: clang/test/Preprocessor/init-x86.c
===
--- clang/test/Preprocessor/init-x86.c
+++ clang/test/Preprocessor/init-x86.c
@@ -185,9 +185,9 @@
 // I386:#define __i386__ 1
 // I386:#define i386 1
 
-// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-ALIGN32 %s
-// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-CXX 
-check-prefix I386-LINUX-ALIGN32 %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 -malign-double < /dev/null | 
FileCheck -match-full-lines -check-prefix I386-LINUX -check-prefix 
I386-LINUX-ALIGN64 %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu i486 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-ALIGN32 %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-ALIGN64 %s
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-ALIGN64 
-check-prefix I386-LINUX-CXX %s
 //
 // I386-LINUX-NOT:#define _LP64
 // I386-LINUX:#define __BIGGEST_ALIGNMENT__ 16
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -298,12 +298,12 @@
 
 /// Get the value the ATOMIC_*_LOCK_FREE macro should have for a type with
 /// the specified properties.
-static const char *getLockFreeValue(unsigned TypeWidth, unsigned TypeAlign,
-unsigned InlineWidth) {
+static const char *getLockFreeValue(unsigned TypeWidth, unsigned InlineWidth) {
   // Fully-aligned, power-of-2 sizes no larger than the inline
   // width will be inlined as lock-free operations.
-  if (TypeWidth == TypeAlign && (TypeWidth & (TypeWidth - 1)) == 0 &&
-  TypeWidth <= InlineWidth)
+  // Note: we do not need to check alignment since _Atomic(T) is always
+  // appropriately-aligned in clang.
+  if ((TypeWidth & (TypeWidth - 1)) == 0 && TypeWidth <= InlineWidth)
 return "2"; // "always lock free"
   // We cannot be certain what operations the lib calls might be
   // able to implement as lock-free on future processors.
@@ -1148,7 +1148,6 @@
 #define DEFINE_LOCK_FREE_MACRO(TYPE, Type) 
\
   Builder.defineMacro(Prefix + #TYPE "_LOCK_FREE", 
\
   getLockFreeValue(TI.get##Type##Width(),  
\
-   TI.get##Type##Align(),  
\
InlineWidthBits));
 DEFINE_LOCK_FREE_MACRO(BOOL, Bool);
 DEFINE_LOCK_FREE_MACRO(CHAR, Char);
@@ -1163,7 +1162,6 @@
 DEFINE_LOCK_FREE_MACRO(LLONG, LongLong);
 Builder.defineMacro(Prefix + "POINTER_LOCK_FREE",
 getLockFreeValue(TI.getPointerWidth(0),
- TI.getPointerAlign(0),
  InlineWidthBits));
 #undef DEFINE_LOCK_FREE_MACRO
   };


Index: clang/test/Sema/atomic-ops.c
===
--- clang/t

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

2022-07-20 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added subscribers: t.p.northover, EricWF, rsmith.
rprichard edited reviewers, added: efriedma, craig.topper; removed: 
t.p.northover, EricWF, rsmith.
rprichard added a comment.
Herald added a subscriber: StephenFan.

@efriedma @craig.topper Could you review this patch (or suggest someone else 
that could)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D28213

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

[PATCH] D127465: [CUDA] Ignore __CLANG_ATOMIC_LLONG_LOCK_FREE on i386

2022-07-21 Thread Ryan Prichard via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG408a2638fda6: [CUDA] Ignore __CLANG_ATOMIC_LLONG_LOCK_FREE 
on i386 (authored by rprichard).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127465

Files:
  clang/test/Preprocessor/cuda-types.cu


Index: clang/test/Preprocessor/cuda-types.cu
===
--- clang/test/Preprocessor/cuda-types.cu
+++ clang/test/Preprocessor/cuda-types.cu
@@ -1,48 +1,53 @@
-// Check that types, widths, __GCC_ATOMIC* macros, etc. match on the host and
+// Check that types, widths, __CLANG_ATOMIC* macros, etc. match on the host and
 // device sides of CUDA compilations.  Note that we filter out long double, as
 // this is intentionally different on host and device.
 //
+// Also ignore __CLANG_ATOMIC_LLONG_LOCK_FREE on i386. The default host CPU for
+// an i386 triple is typically at least an i586, which has cmpxchg8b (Clang
+// feature, "cx8"). Therefore, __CLANG_ATOMIC_LLONG_LOCK_FREE is 2 on the host,
+// but the value should be 1 for the device.
+//
 // FIXME: We really should make __GCC_HAVE_SYNC_COMPARE_AND_SWAP identical on
 // host and device, but architecturally this is difficult at the moment.
 
 // RUN: mkdir -p %t
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
-// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE|_ATOMIC_LLONG_LOCK_FREE' > 
%t/i386-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
-// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE|_ATOMIC_LLONG_LOCK_FREE' > 
%t/i386-device-defines-filtered
 // RUN: diff %t/i386-host-defines-filtered %t/i386-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
 // RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
 // RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
 // RUN: diff %t/x86_64-host-defines-filtered %t/x86_64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target powerpc64-unknown-linux-gnu 
-x cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
 // RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
 // RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
 // RUN: diff %t/powerpc64-host-defines-filtered 
%t/powerpc64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
-// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE|_ATOMIC_LLONG_LOCK_FREE' > 
%t/i386-msvc-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-windows-msvc -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
-// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE|_ATOMIC_LLONG_LOCK_FREE' > 
%t/i386-msvc-device-defines-filte

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

2022-07-21 Thread Ryan Prichard via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG02a25279aedc: [Frontend] Correct values of 
ATOMIC_*_LOCK_FREE to match builtin (authored by rprichard).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D28213

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/init-x86.c
  clang/test/Sema/atomic-ops.c


Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -33,11 +33,7 @@
 _Static_assert(__GCC_ATOMIC_INT_LOCK_FREE == __CLANG_ATOMIC_INT_LOCK_FREE, "");
 _Static_assert(__GCC_ATOMIC_LONG_LOCK_FREE == 2, "");
 _Static_assert(__GCC_ATOMIC_LONG_LOCK_FREE == __CLANG_ATOMIC_LONG_LOCK_FREE, 
"");
-#ifdef __i386__
-_Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == 1, "");
-#else
 _Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == 2, "");
-#endif
 _Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == __CLANG_ATOMIC_LLONG_LOCK_FREE, 
"");
 _Static_assert(__GCC_ATOMIC_POINTER_LOCK_FREE == 2, "");
 _Static_assert(__GCC_ATOMIC_POINTER_LOCK_FREE == 
__CLANG_ATOMIC_POINTER_LOCK_FREE, "");
Index: clang/test/Preprocessor/init-x86.c
===
--- clang/test/Preprocessor/init-x86.c
+++ clang/test/Preprocessor/init-x86.c
@@ -185,9 +185,9 @@
 // I386:#define __i386__ 1
 // I386:#define i386 1
 
-// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-ALIGN32 %s
-// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-CXX 
-check-prefix I386-LINUX-ALIGN32 %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 -malign-double < /dev/null | 
FileCheck -match-full-lines -check-prefix I386-LINUX -check-prefix 
I386-LINUX-ALIGN64 %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu i486 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-ALIGN32 %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-ALIGN64 %s
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-ALIGN64 
-check-prefix I386-LINUX-CXX %s
 //
 // I386-LINUX-NOT:#define _LP64
 // I386-LINUX:#define __BIGGEST_ALIGNMENT__ 16
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -298,12 +298,12 @@
 
 /// Get the value the ATOMIC_*_LOCK_FREE macro should have for a type with
 /// the specified properties.
-static const char *getLockFreeValue(unsigned TypeWidth, unsigned TypeAlign,
-unsigned InlineWidth) {
+static const char *getLockFreeValue(unsigned TypeWidth, unsigned InlineWidth) {
   // Fully-aligned, power-of-2 sizes no larger than the inline
   // width will be inlined as lock-free operations.
-  if (TypeWidth == TypeAlign && (TypeWidth & (TypeWidth - 1)) == 0 &&
-  TypeWidth <= InlineWidth)
+  // Note: we do not need to check alignment since _Atomic(T) is always
+  // appropriately-aligned in clang.
+  if ((TypeWidth & (TypeWidth - 1)) == 0 && TypeWidth <= InlineWidth)
 return "2"; // "always lock free"
   // We cannot be certain what operations the lib calls might be
   // able to implement as lock-free on future processors.
@@ -1142,7 +1142,6 @@
 #define DEFINE_LOCK_FREE_MACRO(TYPE, Type) 
\
   Builder.defineMacro(Prefix + #TYPE "_LOCK_FREE", 
\
   getLockFreeValue(TI.get##Type##Width(),  
\
-   TI.get##Type##Align(),  
\
InlineWidthBits));
 DEFINE_LOCK_FREE_MACRO(BOOL, Bool);
 DEFINE_LOCK_FREE_MACRO(CHAR, Char);
@@ -1157,7 +1156,6 @@
 DEFINE_LOCK_FREE_MACRO(LLONG, LongLong);
 Builder.defineMacro(Prefix + "POINTER_LOCK_FREE",
 getLockFreeValue(TI.getPointerWidth(0),
- TI.getPointerAlign(0),
  InlineWidthBits));
 #undef DEFINE_LOCK_FREE_MACRO
   };


Index: cl

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

2022-05-23 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added subscribers: craig.topper, rprichard.
rprichard added a comment.
Herald added a project: All.

Maybe this change is obsolete now that D59566  
is merged?


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

https://reviews.llvm.org/D29542

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

[PATCH] D93003: [libunwind] LIBUNWIND_HERMETIC_STATIC_LIBRARY fixes

2021-02-05 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard updated this revision to Diff 321701.
rprichard added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Just fix ELF and leave Mach-O as-is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  libunwind/src/assembly.h
  libunwind/src/config.h

Index: libunwind/src/config.h
===
--- libunwind/src/config.h
+++ libunwind/src/config.h
@@ -52,7 +52,7 @@
   #endif
 #endif
 
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
   #define _LIBUNWIND_EXPORT
   #define _LIBUNWIND_HIDDEN
 #else
Index: libunwind/src/assembly.h
===
--- libunwind/src/assembly.h
+++ libunwind/src/assembly.h
@@ -87,17 +87,23 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) .hidden name
+#else
 #define EXPORT_SYMBOL(name)
+#endif
 #define HIDDEN_SYMBOL(name) .hidden name
 #define WEAK_SYMBOL(name) .weak name
 
 #if defined(__hexagon__)
-#define WEAK_ALIAS(name, aliasname) \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+#define WEAK_ALIAS(name, aliasname)\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   .equiv SYMBOL_NAME(aliasname), SYMBOL_NAME(name)
 #else
 #define WEAK_ALIAS(name, aliasname)\
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 #endif
 
@@ -119,7 +125,7 @@
   .section .drectve,"yn" SEPARATOR \
   .ascii "-export:", #name, "\0" SEPARATOR \
   .text
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
 #define EXPORT_SYMBOL(name)
 #else
 #define EXPORT_SYMBOL(name) EXPORT_SYMBOL2(name)
Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -160,11 +160,11 @@
 LINKER_LANGUAGE C
 OUTPUT_NAME "unwind")
 
-  if(LIBUNWIND_HERMETIC_STATIC_LIBRARY)
+  if(LIBUNWIND_HIDE_SYMBOLS)
 append_flags_if_supported(UNWIND_STATIC_LIBRARY_FLAGS -fvisibility=hidden)
 append_flags_if_supported(UNWIND_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
 target_compile_options(unwind_static PRIVATE ${UNWIND_STATIC_LIBRARY_FLAGS})
-target_compile_definitions(unwind_static PRIVATE _LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+target_compile_definitions(unwind_static PRIVATE _LIBUNWIND_HIDE_SYMBOLS)
   endif()
 
   list(APPEND LIBUNWIND_BUILD_TARGETS "unwind_static")
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -101,7 +101,7 @@
   message(FATAL_ERROR "LIBUNWIND_BUILD_32_BITS=ON is not supported on this platform.")
 endif()
 
-option(LIBUNWIND_HERMETIC_STATIC_LIBRARY
+option(LIBUNWIND_HIDE_SYMBOLS
   "Do not export any symbols from the static library." OFF)
 
 #===
@@ -320,7 +320,7 @@
 
 # Disable DLL annotations on Windows for static builds.
 if (WIN32 AND LIBUNWIND_ENABLE_STATIC AND NOT LIBUNWIND_ENABLE_SHARED)
-  add_definitions(-D_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+  add_definitions(-D_LIBUNWIND_HIDE_SYMBOLS)
 endif()
 
 if (LIBUNWIND_HAS_COMMENT_LIB_PRAGMA)
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -179,7 +179,7 @@
 set(RUNTIMES_${target}-unknown-fuchsia_CMAKE_SYSROOT ${FUCHSIA_${target}_SYSROOT} CACHE PATH "")
 set(RUNTIMES_${target}-unknown-fuchsia_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(RUNTIMES_${target}-unknown-fuchsia_LIBUNWIND_HERMETIC_STATIC_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}-unknown-fuchsia_LIBUNWIND_HIDE_SYMBOLS ON CACHE BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia_LIBUNWIND_INSTALL_STATIC_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set

[PATCH] D93003: [libunwind] LIBUNWIND_HERMETIC_STATIC_LIBRARY fixes

2021-02-05 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

After renaming the CMake option, CMake prints a warning if I set the old option:

  CMake Warning:
Manually-specified variables were not used by the project:
  
  LIBUNWIND_HERMETIC_STATIC_LIBRARY


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

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

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

2021-02-08 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added inline comments.



Comment at: libunwind/src/assembly.h:82
   .globl SYMBOL_NAME(aliasname) SEPARATOR  
\
-  WEAK_SYMBOL(aliasname) SEPARATOR 
\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  
\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)

steven_wu wrote:
> rprichard wrote:
> > compnerd wrote:
> > > Does this not change the behaviour on MachO?  This symbol is now 
> > > `private_extern` rather than a `weak_reference`.  A weak reference will 
> > > be set to 0 by the loader if it is not found, and a `private_extern` is a 
> > > strong internal reference.
> > Is `.weak_reference` the right directive to use here, instead of 
> > `.weak_definition`? We're defining a symbol (`aliasname`) and setting its 
> > value to that of another symbol (`name`).
> > 
> > I think marking `unw_*` weak is intended to let some other strong 
> > definition override it. Its value won't ever be set to 0.
> > 
> > Currently on Mach-O, the hide-symbols flag hides almost everything 
> > (including `_Unwind_*`) but leaves all of the `unw_*` alias symbols as 
> > extern (and not private-extern) and not weak. With my change, they're still 
> > not weak, but they're private-extern.
> > 
> > libunwind's current assembly.h behavior for a weak alias:
> > 
> > .globl aliasname
> > .weak_reference aliasname
> > aliasname = name
> > 
> > The LLVM Mach-O assembler ignores the `.weak_reference` directive. If I 
> > change it to `.weak_definition`, it is still ignored. AFAICT, the LLVM 
> > assembler uses the WeakDef/WeakRef attributes from the `name` symbol.
> > 
> > e.g.
> > 
> > ```
> > $ cat test.S
> > .text
> > .space 0x42
> > 
> > // Define foo.
> > .globl foo
> > foo:
> > ret
> > 
> > // Define a weak alias, bar.
> > .globl bar
> > .weak_reference bar
> > bar = foo
> > 
> > $ ~/clang11/bin/clang test.S -c && ~/clang11/bin/llvm-readobj --syms test.o
> > 
> > File: test.o
> > Format: Mach-O 64-bit x86-64
> > Arch: x86_64
> > AddressSize: 64bit
> > Symbols [
> >   Symbol {
> > Name: bar (1)
> > Extern
> > Type: Section (0xE)
> > Section: __text (0x1)
> > RefType: UndefinedNonLazy (0x0)
> > Flags [ (0x0)
> > ]
> > Value: 0x42
> >   }
> >   Symbol {
> > Name: foo (5)
> > Extern
> > Type: Section (0xE)
> > Section: __text (0x1)
> > RefType: UndefinedNonLazy (0x0)
> > Flags [ (0x0)
> > ]
> > Value: 0x42
> >   }
> > ]
> > ```
> > 
> > The Flags are empty.
> > 
> > OTOH, if I flip things around:
> > 
> > ```
> > .text
> > .space 0x42
> > 
> > // Define a weak function, foo.
> > .globl foo
> > .weak_reference foo
> > foo:
> > ret
> > 
> > // Define an alias, bar.
> > .globl bar
> > bar = foo
> > ```
> > 
> > Now both symbols are WeakRef:
> > 
> > ```
> > File: test.o
> > Format: Mach-O 64-bit x86-64
> > Arch: x86_64
> > AddressSize: 64bit
> > Symbols [
> >   Symbol {
> > Name: bar (1)
> > Extern
> > Type: Section (0xE)
> > Section: __text (0x1)
> > RefType: UndefinedNonLazy (0x0)
> > Flags [ (0x40)
> >   WeakRef (0x40)
> > ]
> > Value: 0x42
> >   }
> >   Symbol {
> > Name: foo (5)
> > Extern
> > Type: Section (0xE)
> > Section: __text (0x1)
> > RefType: UndefinedNonLazy (0x0)
> > Flags [ (0x40)
> >   WeakRef (0x40)
> > ]
> > Value: 0x42
> >   }
> > ]
> > ```
> > 
> > I'm wondering if this LLVM behavior is actually correct, but I'm also 
> > unfamiliar with Mach-O. (i.e. We want to copy the symbol's address, but 
> > should we be copying the WeakRef/WeakDef flags?) It looks like the behavior 
> > is controlled in `MachObjectWriter::writeNlist`. `writeNlist` finds the 
> > aliased symbol and uses its flags:
> > ```
> >   // The Mach-O streamer uses the lowest 16-bits of the flags for the 'desc'
> >   // value.
> >   bool EncodeAsAltEntry =
> > IsAlias && cast(OrigSymbol).isAltEntry();
> >   
> > W.write(cast(Symbol)->getEncodedFlags(EncodeAsAltEntry));
> > ```
> > 
> > The PrivateExtern attribute, on the other hand, isn't part of these encoded 
> > flags:
> > ```
> >   if (Data.isPrivateExtern())
> > Type |= MachO::N_PEXT;
> > ```
> > `Data` continues to refer to the alias symbol rather than the aliased 
> > symbol. However, apparently the author isn't completely sure about things?
> > ```
> > // FIXME: Should this update Data as well?
> > ```
> > 
> > In libunwind, there is one usage of assembly.h's WEAK_ALIAS. 
> > UnwindRegistersSave.S defines a hidden non-weak __unw_getcontext function, 
> > and also a weak alias unw_getcontext. My patch's goal is to make 
> > unw_getcontext hidden or not-hidden depending on a CMake config variable.
> > 
> > Currently, on Mach-O and on Windows, `WEAK_ALIAS` doesn't actually make the 
> > alias weak. I'm just making it a

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

2021-02-09 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard updated this revision to Diff 322574.
rprichard edited the summary of this revision.
rprichard added a comment.

Restore Mach-O alias fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  libunwind/src/assembly.h
  libunwind/src/config.h

Index: libunwind/src/config.h
===
--- libunwind/src/config.h
+++ libunwind/src/config.h
@@ -52,7 +52,8 @@
   #endif
 #endif
 
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+  // The CMake file passes -fvisibility=hidden to control ELF/Mach-O visibility.
   #define _LIBUNWIND_EXPORT
   #define _LIBUNWIND_HIDDEN
 #else
@@ -70,11 +71,15 @@
 #define SYMBOL_NAME(name) XSTR(__USER_LABEL_PREFIX__) #name
 
 #if defined(__APPLE__)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define _LIBUNWIND_ALIAS_VISIBILITY(name) __asm__(".private_extern " name)
+#else
+#define _LIBUNWIND_ALIAS_VISIBILITY(name)
+#endif
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   __asm__(".globl " SYMBOL_NAME(aliasname));   \
   __asm__(SYMBOL_NAME(aliasname) " = " SYMBOL_NAME(name)); \
-  extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
-  __attribute__((weak_import));
+  _LIBUNWIND_ALIAS_VISIBILITY(SYMBOL_NAME(aliasname));
 #elif defined(__ELF__)
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
Index: libunwind/src/assembly.h
===
--- libunwind/src/assembly.h
+++ libunwind/src/assembly.h
@@ -70,12 +70,15 @@
 #if defined(__APPLE__)
 
 #define SYMBOL_IS_FUNC(name)
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .private_extern name
-#define WEAK_SYMBOL(name) .weak_reference name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_ALIAS(name, aliasname)\
   .globl SYMBOL_NAME(aliasname) SEPARATOR  \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 
 #define NO_EXEC_STACK_DIRECTIVE
@@ -87,17 +90,23 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .hidden name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_SYMBOL(name) .weak name
 
 #if defined(__hexagon__)
-#define WEAK_ALIAS(name, aliasname) \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+#define WEAK_ALIAS(name, aliasname)\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   .equiv SYMBOL_NAME(aliasname), SYMBOL_NAME(name)
 #else
 #define WEAK_ALIAS(name, aliasname)\
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 #endif
 
@@ -119,7 +128,7 @@
   .section .drectve,"yn" SEPARATOR \
   .ascii "-export:", #name, "\0" SEPARATOR \
   .text
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
 #define EXPORT_SYMBOL(name)
 #else
 #define EXPORT_SYMBOL(name) EXPORT_SYMBOL2(name)
Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -160,11 +160,11 @@
 LINKER_LANGUAGE C
 OUTPUT_NAME "unwind")
 
-  if(LIBUNWIND_HERMETIC_STATIC_LIBRARY)
+  if(LIBUNWIND_HIDE_SYMBOLS)
 append_flags_if_supported(UNWIND_STATIC_LIBRARY_FLAGS -fvisibility=hidden)
 append_flags_if_supported(UNWIND_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
 target_compile_options(unwind_static PRIVATE ${UNWIND_STATIC_LIBRARY_FLAGS})
-target_compile_definitions(unwind_static PRIVATE _LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+target_compile_definitions(unwind_static PRIVATE _LIBUNWIND_HIDE_SYMBOLS)
   endif()
 
   list(APPEND LIBUNWIND_BU

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

2021-02-09 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added inline comments.



Comment at: libunwind/src/assembly.h:82
   .globl SYMBOL_NAME(aliasname) SEPARATOR  
\
-  WEAK_SYMBOL(aliasname) SEPARATOR 
\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  
\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)

rprichard wrote:
> steven_wu wrote:
> > rprichard wrote:
> > > compnerd wrote:
> > > > Does this not change the behaviour on MachO?  This symbol is now 
> > > > `private_extern` rather than a `weak_reference`.  A weak reference will 
> > > > be set to 0 by the loader if it is not found, and a `private_extern` is 
> > > > a strong internal reference.
> > > Is `.weak_reference` the right directive to use here, instead of 
> > > `.weak_definition`? We're defining a symbol (`aliasname`) and setting its 
> > > value to that of another symbol (`name`).
> > > 
> > > I think marking `unw_*` weak is intended to let some other strong 
> > > definition override it. Its value won't ever be set to 0.
> > > 
> > > Currently on Mach-O, the hide-symbols flag hides almost everything 
> > > (including `_Unwind_*`) but leaves all of the `unw_*` alias symbols as 
> > > extern (and not private-extern) and not weak. With my change, they're 
> > > still not weak, but they're private-extern.
> > > 
> > > libunwind's current assembly.h behavior for a weak alias:
> > > 
> > > .globl aliasname
> > > .weak_reference aliasname
> > > aliasname = name
> > > 
> > > The LLVM Mach-O assembler ignores the `.weak_reference` directive. If I 
> > > change it to `.weak_definition`, it is still ignored. AFAICT, the LLVM 
> > > assembler uses the WeakDef/WeakRef attributes from the `name` symbol.
> > > 
> > > e.g.
> > > 
> > > ```
> > > $ cat test.S
> > > .text
> > > .space 0x42
> > > 
> > > // Define foo.
> > > .globl foo
> > > foo:
> > > ret
> > > 
> > > // Define a weak alias, bar.
> > > .globl bar
> > > .weak_reference bar
> > > bar = foo
> > > 
> > > $ ~/clang11/bin/clang test.S -c && ~/clang11/bin/llvm-readobj --syms 
> > > test.o
> > > 
> > > File: test.o
> > > Format: Mach-O 64-bit x86-64
> > > Arch: x86_64
> > > AddressSize: 64bit
> > > Symbols [
> > >   Symbol {
> > > Name: bar (1)
> > > Extern
> > > Type: Section (0xE)
> > > Section: __text (0x1)
> > > RefType: UndefinedNonLazy (0x0)
> > > Flags [ (0x0)
> > > ]
> > > Value: 0x42
> > >   }
> > >   Symbol {
> > > Name: foo (5)
> > > Extern
> > > Type: Section (0xE)
> > > Section: __text (0x1)
> > > RefType: UndefinedNonLazy (0x0)
> > > Flags [ (0x0)
> > > ]
> > > Value: 0x42
> > >   }
> > > ]
> > > ```
> > > 
> > > The Flags are empty.
> > > 
> > > OTOH, if I flip things around:
> > > 
> > > ```
> > > .text
> > > .space 0x42
> > > 
> > > // Define a weak function, foo.
> > > .globl foo
> > > .weak_reference foo
> > > foo:
> > > ret
> > > 
> > > // Define an alias, bar.
> > > .globl bar
> > > bar = foo
> > > ```
> > > 
> > > Now both symbols are WeakRef:
> > > 
> > > ```
> > > File: test.o
> > > Format: Mach-O 64-bit x86-64
> > > Arch: x86_64
> > > AddressSize: 64bit
> > > Symbols [
> > >   Symbol {
> > > Name: bar (1)
> > > Extern
> > > Type: Section (0xE)
> > > Section: __text (0x1)
> > > RefType: UndefinedNonLazy (0x0)
> > > Flags [ (0x40)
> > >   WeakRef (0x40)
> > > ]
> > > Value: 0x42
> > >   }
> > >   Symbol {
> > > Name: foo (5)
> > > Extern
> > > Type: Section (0xE)
> > > Section: __text (0x1)
> > > RefType: UndefinedNonLazy (0x0)
> > > Flags [ (0x40)
> > >   WeakRef (0x40)
> > > ]
> > > Value: 0x42
> > >   }
> > > ]
> > > ```
> > > 
> > > I'm wondering if this LLVM behavior is actually correct, but I'm also 
> > > unfamiliar with Mach-O. (i.e. We want to copy the symbol's address, but 
> > > should we be copying the WeakRef/WeakDef flags?) It looks like the 
> > > behavior is controlled in `MachObjectWriter::writeNlist`. `writeNlist` 
> > > finds the aliased symbol and uses its flags:
> > > ```
> > >   // The Mach-O streamer uses the lowest 16-bits of the flags for the 
> > > 'desc'
> > >   // value.
> > >   bool EncodeAsAltEntry =
> > > IsAlias && cast(OrigSymbol).isAltEntry();
> > >   
> > > W.write(cast(Symbol)->getEncodedFlags(EncodeAsAltEntry));
> > > ```
> > > 
> > > The PrivateExtern attribute, on the other hand, isn't part of these 
> > > encoded flags:
> > > ```
> > >   if (Data.isPrivateExtern())
> > > Type |= MachO::N_PEXT;
> > > ```
> > > `Data` continues to refer to the alias symbol rather than the aliased 
> > > symbol. However, apparently the author isn't completely sure about things?
> > > ```
> > > // FIXME: Should this update Data as well?
> > > ```
> > > 
> > > In libunwind, there is one usage of assembly.h's WEAK_ALIAS. 
> > > Unwi

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

2021-02-09 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a subscriber: emaste.
rprichard added a comment.

emaste: Just a heads up in case FreeBSD is affected. FWIW, I noticed that 
libgcc_eh.a on FreeBSD 12.1 doesn't hide the `_Unwind_*` or `unw_*` symbols. 
The build system 

 is defining `VISIBILITY_HIDDEN` but libunwind doesn't respond to that name. 
FreeBSD libgcc_eh.a also defines hidden symbols named `logAPIs`, 
`logUnwinding`, and `logDWARF`. libunwind defines these internal names if 
`NDEBUG` isn't defined. They can break statically-linked programs on FreeBSD 
(e.g. duplicate symbol linker errors).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

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

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

2021-02-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard created this revision.
Herald added a subscriber: dberris.
rprichard requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added a project: clang.

On Android, the unwinder isn't part of the C++ STL and isn't (in older
versions) exported from libc.so. Instead, the driver links the static
unwinder archive implicitly. Currently, the Android NDK implicitly
links libgcc.a to provide both builtins and the unwinder.

To support switching to compiler-rt builtins and libunwind, make
--rtlib=compiler-rt behave the same way on Android, and implicitly pass
-l:libunwind.a to the linker.

Adjust the -ldl logic. For the Android NDK, the unwinder (whether
libgcc.a or libunwind.a) is linked statically and calls a function in
the dynamic loader for finding unwind tables (e.g. dl_iterate_phdr).
On Android, this function is in libc.a for static executables and
libdl.so otherwise, so -ldl is needed. (glibc doesn't need -ldl because
its libc.so exports dl_iterate_phdr.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96403

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1366,11 +1366,17 @@
 
 enum class LibGccType { UnspecifiedLibGcc, StaticLibGcc, SharedLibGcc };
 
-static LibGccType getLibGccType(const Driver &D, const ArgList &Args) {
+static LibGccType getLibGccType(const ToolChain &TC, const Driver &D,
+const ArgList &Args) {
   if (Args.hasArg(options::OPT_static_libgcc) ||
   Args.hasArg(options::OPT_static) || Args.hasArg(options::OPT_static_pie))
 return LibGccType::StaticLibGcc;
-  if (Args.hasArg(options::OPT_shared_libgcc) || D.CCCIsCXX())
+  if (Args.hasArg(options::OPT_shared_libgcc))
+return LibGccType::SharedLibGcc;
+  // The Android NDK only provides libunwind.a, not libunwind.so.
+  if (TC.getTriple().isAndroid())
+return LibGccType::StaticLibGcc;
+  if (D.CCCIsCXX())
 return LibGccType::SharedLibGcc;
   return LibGccType::UnspecifiedLibGcc;
 }
@@ -1392,12 +1398,12 @@
  ArgStringList &CmdArgs, const ArgList &Args) {
   ToolChain::UnwindLibType UNW = TC.GetUnwindLibType(Args);
   // Targets that don't use unwind libraries.
-  if (TC.getTriple().isAndroid() || TC.getTriple().isOSIAMCU() ||
-  TC.getTriple().isOSBinFormatWasm() ||
+  if ((TC.getTriple().isAndroid() && UNW == ToolChain::UNW_Libgcc) ||
+  TC.getTriple().isOSIAMCU() || TC.getTriple().isOSBinFormatWasm() ||
   UNW == ToolChain::UNW_None)
 return;
 
-  LibGccType LGT = getLibGccType(D, Args);
+  LibGccType LGT = getLibGccType(TC, D, Args);
   bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
   !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
   if (AsNeeded)
@@ -1434,20 +1440,12 @@
 
 static void AddLibgcc(const ToolChain &TC, const Driver &D,
   ArgStringList &CmdArgs, const ArgList &Args) {
-  LibGccType LGT = getLibGccType(D, Args);
+  LibGccType LGT = getLibGccType(TC, D, Args);
   if (LGT != LibGccType::SharedLibGcc)
 CmdArgs.push_back("-lgcc");
   AddUnwindLibrary(TC, D, CmdArgs, Args);
   if (LGT == LibGccType::SharedLibGcc)
 CmdArgs.push_back("-lgcc");
-
-  // According to Android ABI, we have to link with libdl if we are
-  // linking with non-static libgcc.
-  //
-  // NOTE: This fixes a link error on Android MIPS as well.  The non-static
-  // libgcc for MIPS relies on _Unwind_Find_FDE and dl_iterate_phdr from libdl.
-  if (TC.getTriple().isAndroid() && LGT != LibGccType::StaticLibGcc)
-CmdArgs.push_back("-ldl");
 }
 
 void tools::AddRunTimeLibs(const ToolChain &TC, const Driver &D,
@@ -1473,6 +1471,13 @@
   AddLibgcc(TC, D, CmdArgs, Args);
 break;
   }
+
+  // On Android, the unwinder uses dl_iterate_phdr (or one of
+  // dl_unwind_find_exidx/__gnu_Unwind_Find_exidx on arm32) from libdl.so. For
+  // statically-linked executables, these functions come from libc.a instead.
+  if (TC.getTriple().isAndroid() && !Args.hasArg(options::OPT_static) &&
+  !Args.hasArg(options::OPT_static_pie))
+CmdArgs.push_back("-ldl");
 }
 
 SmallString<128> tools::getStatsFileName(const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -920,9 +920,12 @@
 unwindLibType = ToolChain::UNW_None;
   else if (LibName == "platform" || LibName == "") {
 ToolChain::RuntimeLibType RtLibType = GetRuntimeLibType(Args);
-if (RtLibType == ToolChain::RLT_CompilerRT)
-  unwindLibType = ToolChain::UNW_None;
-else if (RtLibType == ToolChain::RLT_Libgcc)
+if (RtLibType == ToolChain::RLT_CompilerRT) {
+  if (g

[PATCH] D96404: [Android] Default to --rtlib=compiler-rt

2021-02-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard created this revision.
Herald added a subscriber: dberris.
rprichard requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

By default, the driver uses the compiler-rt builtins and links with
-l:libunwind.a.

Restore the previous behavior by passing --rtlib=libgcc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96404

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/Linux.h
  clang/test/Driver/linux-ld.c

Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -288,7 +288,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-ANDROID-NONE %s
 // CHECK-CLANG-ANDROID-NONE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-CLANG-ANDROID-NONE: "-lgcc" "-ldl" "-lc"
+// CHECK-CLANG-ANDROID-NONE: "-l:libunwind.a" "-ldl" "-lc"
 //
 // RUN: %clang -shared -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=aarch64-linux-android -rtlib=platform \
@@ -296,7 +296,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-ANDROID-SHARED %s
 // CHECK-CLANG-ANDROID-SHARED: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-CLANG-ANDROID-SHARED: "-lgcc" "-ldl" "-lc"
+// CHECK-CLANG-ANDROID-SHARED: "-l:libunwind.a" "-ldl" "-lc"
 //
 // RUN: %clang -static -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=aarch64-linux-android -rtlib=platform \
@@ -304,7 +304,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-ANDROID-STATIC %s
 // CHECK-CLANG-ANDROID-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-CLANG-ANDROID-STATIC: "--start-group" "-lgcc" "-lc" "--end-group"
+// CHECK-CLANG-ANDROID-STATIC: "--start-group" "{{[^"]*}}{{/|}}libclang_rt.builtins-aarch64-android.a" "-l:libunwind.a" "-lc" "--end-group"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1  \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
@@ -1353,10 +1353,12 @@
 // CHECK-ANDROID: "--enable-new-dtags"
 // CHECK-ANDROID: "{{.*}}{{/|}}crtbegin_dynamic.o"
 // CHECK-ANDROID: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-NOT: "gcc_s"
-// CHECK-ANDROID: "-lgcc"
+// CHECK-ANDROID-NOT: "-lgcc_s"
+// CHECK-ANDROID-NOT: "-lgcc"
+// CHECK-ANDROID: "-l:libunwind.a"
 // CHECK-ANDROID: "-ldl"
-// CHECK-ANDROID-NOT: "gcc_s"
+// CHECK-ANDROID-NOT: "-lgcc_s"
+// CHECK-ANDROID-NOT: "-lgcc"
 // CHECK-ANDROID: "{{.*}}{{/|}}crtend_android.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi -rtlib=platform \
@@ -1409,10 +1411,12 @@
 // CHECK-ANDROID-SO-NOT: "-Bsymbolic"
 // CHECK-ANDROID-SO: "{{.*}}{{/|}}crtbegin_so.o"
 // CHECK-ANDROID-SO: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-SO-NOT: "gcc_s"
-// CHECK-ANDROID-SO: "-lgcc"
+// CHECK-ANDROID-SO-NOT: "-lgcc_s"
+// CHECK-ANDROID-SO-NOT: "-lgcc"
+// CHECK-ANDROID-SO: "-l:libunwind.a"
 // CHECK-ANDROID-SO: "-ldl"
-// CHECK-ANDROID-SO-NOT: "gcc_s"
+// CHECK-ANDROID-SO-NOT: "-lgcc_s"
+// CHECK-ANDROID-SO-NOT: "-lgcc"
 // CHECK-ANDROID-SO: "{{.*}}{{/|}}crtend_so.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi -rtlib=platform \
@@ -1463,10 +1467,12 @@
 // CHECK-ANDROID-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ANDROID-STATIC: "{{.*}}{{/|}}crtbegin_static.o"
 // CHECK-ANDROID-STATIC: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-STATIC-NOT: "gcc_s"
-// CHECK-ANDROID-STATIC: "-lgcc"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc_eh"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc"
+// CHECK-ANDROID-STATIC: "-l:libunwind.a"
 // CHECK-ANDROID-STATIC-NOT: "-ldl"
-// CHECK-ANDROID-STATIC-NOT: "gcc_s"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc_eh"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc"
 // CHECK-ANDROID-STATIC: "{{.*}}{{/|}}crtend_android.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi -rtlib=platform \
@@ -1519,9 +1525,11 @@
 // CHECK-ANDROID-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ANDROID-PIE: "{{.*}}{{/|}}crtbegin_dynamic.o"
 // CHECK-ANDROID-PIE: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-PIE-NOT: "gcc_s"
-// CHECK-ANDROID-PIE: "-lgcc"
-// CHECK-ANDROID-PIE-NOT: "gcc_s"
+// CHECK-ANDROID-PIE-NOT: "-lgcc_s"
+// CHECK-ANDROID-PIE-NOT: "-lgcc"
+// CHECK-ANDROID-PIE: "-l:libunwind.a"
+// CHECK-ANDROID-PIE-NOT: "-lgcc_s"
+// CHECK-ANDROID-PIE-NOT: "-lgcc"
 // CHECK-ANDROID-PIE: "{{.*}}{{/|}}crtend_android.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi \
Index: clang/lib/Driver/ToolChains/Linux.h
===
--- clang/li

[PATCH] D96404: [Android] Default to --rtlib=compiler-rt

2021-02-10 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added reviewers: danalbert, srhines, pcc.
rprichard added subscribers: thakis, glandium.
rprichard added a comment.

Adding glandium and thakis for Firefox and Chrome. e.g. I suspect this change 
would cause the same sort of breakage seen in D95166 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96404

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

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

2021-02-11 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard updated this revision to Diff 323221.
rprichard edited the summary of this revision.
rprichard added a comment.

Update libunwind/src/BUILD.gn for macro name change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  libunwind/src/assembly.h
  libunwind/src/config.h
  llvm/utils/gn/secondary/libunwind/src/BUILD.gn

Index: llvm/utils/gn/secondary/libunwind/src/BUILD.gn
===
--- llvm/utils/gn/secondary/libunwind/src/BUILD.gn
+++ llvm/utils/gn/secondary/libunwind/src/BUILD.gn
@@ -98,7 +98,7 @@
 if (libunwind_hermetic_static_library) {
   cflags = [ "-fvisibility=hidden" ]
   cflags_cc = [ "-fvisibility-global-new-delete-hidden" ]
-  defines = [ "_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS" ]
+  defines = [ "_LIBUNWIND_HIDE_SYMBOLS" ]
 }
 deps = [ "//compiler-rt/lib/builtins" ]
 configs += [ ":unwind_config" ]
Index: libunwind/src/config.h
===
--- libunwind/src/config.h
+++ libunwind/src/config.h
@@ -52,7 +52,8 @@
   #endif
 #endif
 
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+  // The CMake file passes -fvisibility=hidden to control ELF/Mach-O visibility.
   #define _LIBUNWIND_EXPORT
   #define _LIBUNWIND_HIDDEN
 #else
@@ -70,11 +71,15 @@
 #define SYMBOL_NAME(name) XSTR(__USER_LABEL_PREFIX__) #name
 
 #if defined(__APPLE__)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define _LIBUNWIND_ALIAS_VISIBILITY(name) __asm__(".private_extern " name)
+#else
+#define _LIBUNWIND_ALIAS_VISIBILITY(name)
+#endif
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   __asm__(".globl " SYMBOL_NAME(aliasname));   \
   __asm__(SYMBOL_NAME(aliasname) " = " SYMBOL_NAME(name)); \
-  extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
-  __attribute__((weak_import));
+  _LIBUNWIND_ALIAS_VISIBILITY(SYMBOL_NAME(aliasname));
 #elif defined(__ELF__)
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
Index: libunwind/src/assembly.h
===
--- libunwind/src/assembly.h
+++ libunwind/src/assembly.h
@@ -70,12 +70,15 @@
 #if defined(__APPLE__)
 
 #define SYMBOL_IS_FUNC(name)
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .private_extern name
-#define WEAK_SYMBOL(name) .weak_reference name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_ALIAS(name, aliasname)\
   .globl SYMBOL_NAME(aliasname) SEPARATOR  \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 
 #define NO_EXEC_STACK_DIRECTIVE
@@ -87,17 +90,23 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .hidden name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_SYMBOL(name) .weak name
 
 #if defined(__hexagon__)
-#define WEAK_ALIAS(name, aliasname) \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+#define WEAK_ALIAS(name, aliasname)\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   .equiv SYMBOL_NAME(aliasname), SYMBOL_NAME(name)
 #else
 #define WEAK_ALIAS(name, aliasname)\
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 #endif
 
@@ -119,7 +128,7 @@
   .section .drectve,"yn" SEPARATOR \
   .ascii "-export:", #name, "\0" SEPARATOR \
   .text
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
 #define EXPORT_SYMBOL(name)
 #else
 #define EXPORT_SYMBOL(name) EXPORT_SYMBOL2(name)
Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/C

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

2021-02-11 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard updated this revision to Diff 323222.
rprichard added a comment.

Rebase and fix merge conflict in gn file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  libunwind/src/assembly.h
  libunwind/src/config.h
  llvm/utils/gn/secondary/libunwind/src/BUILD.gn

Index: llvm/utils/gn/secondary/libunwind/src/BUILD.gn
===
--- llvm/utils/gn/secondary/libunwind/src/BUILD.gn
+++ llvm/utils/gn/secondary/libunwind/src/BUILD.gn
@@ -111,7 +111,7 @@
   if (!invoker.export) {
 cflags = [ "-fvisibility=hidden" ]
 cflags_cc = [ "-fvisibility-global-new-delete-hidden" ]
-defines = [ "_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS" ]
+defines = [ "_LIBUNWIND_HIDE_SYMBOLS" ]
   }
   deps = [ "//compiler-rt/lib/builtins" ]
   configs += [ ":unwind_config" ]
Index: libunwind/src/config.h
===
--- libunwind/src/config.h
+++ libunwind/src/config.h
@@ -52,7 +52,8 @@
   #endif
 #endif
 
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+  // The CMake file passes -fvisibility=hidden to control ELF/Mach-O visibility.
   #define _LIBUNWIND_EXPORT
   #define _LIBUNWIND_HIDDEN
 #else
@@ -70,11 +71,15 @@
 #define SYMBOL_NAME(name) XSTR(__USER_LABEL_PREFIX__) #name
 
 #if defined(__APPLE__)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define _LIBUNWIND_ALIAS_VISIBILITY(name) __asm__(".private_extern " name)
+#else
+#define _LIBUNWIND_ALIAS_VISIBILITY(name)
+#endif
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   __asm__(".globl " SYMBOL_NAME(aliasname));   \
   __asm__(SYMBOL_NAME(aliasname) " = " SYMBOL_NAME(name)); \
-  extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
-  __attribute__((weak_import));
+  _LIBUNWIND_ALIAS_VISIBILITY(SYMBOL_NAME(aliasname));
 #elif defined(__ELF__)
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
Index: libunwind/src/assembly.h
===
--- libunwind/src/assembly.h
+++ libunwind/src/assembly.h
@@ -70,12 +70,15 @@
 #if defined(__APPLE__)
 
 #define SYMBOL_IS_FUNC(name)
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .private_extern name
-#define WEAK_SYMBOL(name) .weak_reference name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_ALIAS(name, aliasname)\
   .globl SYMBOL_NAME(aliasname) SEPARATOR  \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 
 #define NO_EXEC_STACK_DIRECTIVE
@@ -87,17 +90,23 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .hidden name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_SYMBOL(name) .weak name
 
 #if defined(__hexagon__)
-#define WEAK_ALIAS(name, aliasname) \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+#define WEAK_ALIAS(name, aliasname)\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   .equiv SYMBOL_NAME(aliasname), SYMBOL_NAME(name)
 #else
 #define WEAK_ALIAS(name, aliasname)\
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 #endif
 
@@ -119,7 +128,7 @@
   .section .drectve,"yn" SEPARATOR \
   .ascii "-export:", #name, "\0" SEPARATOR \
   .text
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
 #define EXPORT_SYMBOL(name)
 #else
 #define EXPORT_SYMBOL(name) EXPORT_SYMBOL2(name)
Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -160,11

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

2021-02-16 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

Maybe this is blocked on someone from Apple reviewing the Mach-O parts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

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

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

2021-02-19 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

For reference, I know of four ways that LLVM's libunwind is currently linked 
into programs:

- Fuchsia/Windows (i.e. Clang's default behavior): it's part of libc++, e.g.:
  - Fuchsia:
- 
`fuchsia/prebuilt/third_party/clang/linux-x64/lib/x86_64-unknown-fuchsia/c++/libc++.so`
 is `INPUT(libc++.so.2 -lunwind -lc++abi)`.
- 
`fuchsia/prebuilt/third_party/clang/linux-x64/lib/x86_64-unknown-fuchsia/c++/libc++.a`
 is an archive that contains the libunwind object files.
  - In https://github.com/mstorsjo/llvm-mingw/releases/tag/20201020:
- `llvm-mingw\x86_64-w64-mingw32\bin\libc++.dll` uses `_Unwind_Resume` from 
libunwind.dll
- `llvm-mingw\x86_64-w64-mingw32\lib\libc++.dll.a` also seems to import 
`_Unwind_Resume` from libunwind.dll
- `llvm-mingw\x86_64-w64-mingw32\lib\libc++.a` has the libunwind object 
files in it
- FreeBSD: libclang_rt.builtins and libunwind are repackaged into libraries 
that look like libgcc, linked implicitly.
- Darwin: it's part of libSystem, linked implicitly.
- crosstool in google3: I think this build system is passing 
--unwindlib=libunwind to the driver.

The Android team's LLVM build is used to target several OS's: host/glibc Linux, 
Windows, Darwin, and Android. At some point, we'd also like Android NDK to rely 
on the unwinder exported from libc.so (when the API level is high enough), 
which prevents us from using Clang's `CLANG_DEFAULT_UNWINDLIB` config setting.

Even for C programs, the unwinder has at least a couple of uses I know of:

- `_Unwind_Backtrace`
- `__attribute__((cleanup))` with `-fexceptions`

I don't really know how important these use cases are, or whether there are 
other important use cases I'm not thinking of. I'd prefer to keep linking the 
unwinder implicitly.

See D57128  and D59109 
.

I think the Android team is happy with this driver change, though, so I expect 
I'll push it next week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96403

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

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

2021-02-22 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

The coff-dwarf.test buildbot failures were fixed by 
https://github.com/llvm/llvm-project/commit/0fd7c31a098efdfaa5a57dbac6e9c0921b00a999
 (on Feb 11).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

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

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

2021-02-22 Thread Ryan Prichard via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG729899f7b6bf: [libunwind] unw_* alias fixes for ELF and 
Mach-O (authored by rprichard).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  libunwind/src/assembly.h
  libunwind/src/config.h
  llvm/utils/gn/secondary/libunwind/src/BUILD.gn

Index: llvm/utils/gn/secondary/libunwind/src/BUILD.gn
===
--- llvm/utils/gn/secondary/libunwind/src/BUILD.gn
+++ llvm/utils/gn/secondary/libunwind/src/BUILD.gn
@@ -111,7 +111,7 @@
   if (!invoker.export) {
 cflags = [ "-fvisibility=hidden" ]
 cflags_cc = [ "-fvisibility-global-new-delete-hidden" ]
-defines = [ "_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS" ]
+defines = [ "_LIBUNWIND_HIDE_SYMBOLS" ]
   }
   deps = [ "//compiler-rt/lib/builtins" ]
   configs += [ ":unwind_config" ]
Index: libunwind/src/config.h
===
--- libunwind/src/config.h
+++ libunwind/src/config.h
@@ -52,7 +52,8 @@
   #endif
 #endif
 
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+  // The CMake file passes -fvisibility=hidden to control ELF/Mach-O visibility.
   #define _LIBUNWIND_EXPORT
   #define _LIBUNWIND_HIDDEN
 #else
@@ -70,11 +71,15 @@
 #define SYMBOL_NAME(name) XSTR(__USER_LABEL_PREFIX__) #name
 
 #if defined(__APPLE__)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define _LIBUNWIND_ALIAS_VISIBILITY(name) __asm__(".private_extern " name)
+#else
+#define _LIBUNWIND_ALIAS_VISIBILITY(name)
+#endif
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   __asm__(".globl " SYMBOL_NAME(aliasname));   \
   __asm__(SYMBOL_NAME(aliasname) " = " SYMBOL_NAME(name)); \
-  extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
-  __attribute__((weak_import));
+  _LIBUNWIND_ALIAS_VISIBILITY(SYMBOL_NAME(aliasname));
 #elif defined(__ELF__)
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
Index: libunwind/src/assembly.h
===
--- libunwind/src/assembly.h
+++ libunwind/src/assembly.h
@@ -70,12 +70,15 @@
 #if defined(__APPLE__)
 
 #define SYMBOL_IS_FUNC(name)
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .private_extern name
-#define WEAK_SYMBOL(name) .weak_reference name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_ALIAS(name, aliasname)\
   .globl SYMBOL_NAME(aliasname) SEPARATOR  \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 
 #define NO_EXEC_STACK_DIRECTIVE
@@ -87,17 +90,23 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .hidden name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_SYMBOL(name) .weak name
 
 #if defined(__hexagon__)
-#define WEAK_ALIAS(name, aliasname) \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+#define WEAK_ALIAS(name, aliasname)\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   .equiv SYMBOL_NAME(aliasname), SYMBOL_NAME(name)
 #else
 #define WEAK_ALIAS(name, aliasname)\
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 #endif
 
@@ -119,7 +128,7 @@
   .section .drectve,"yn" SEPARATOR \
   .ascii "-export:", #name, "\0" SEPARATOR \
   .text
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
 #define EXPORT_SYMBOL(name)
 #else
 #define EXPORT_SYMBOL(name) EXPORT_SYMBOL2(name)
Index: libunwind/src/CMakeLists.txt
==

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

2021-02-25 Thread Ryan Prichard via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG91f8aacc040f: [Android] Use -l:libunwind.a with 
--rtlib=compiler-rt (authored by rprichard).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96403

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1366,11 +1366,17 @@
 
 enum class LibGccType { UnspecifiedLibGcc, StaticLibGcc, SharedLibGcc };
 
-static LibGccType getLibGccType(const Driver &D, const ArgList &Args) {
+static LibGccType getLibGccType(const ToolChain &TC, const Driver &D,
+const ArgList &Args) {
   if (Args.hasArg(options::OPT_static_libgcc) ||
   Args.hasArg(options::OPT_static) || Args.hasArg(options::OPT_static_pie))
 return LibGccType::StaticLibGcc;
-  if (Args.hasArg(options::OPT_shared_libgcc) || D.CCCIsCXX())
+  if (Args.hasArg(options::OPT_shared_libgcc))
+return LibGccType::SharedLibGcc;
+  // The Android NDK only provides libunwind.a, not libunwind.so.
+  if (TC.getTriple().isAndroid())
+return LibGccType::StaticLibGcc;
+  if (D.CCCIsCXX())
 return LibGccType::SharedLibGcc;
   return LibGccType::UnspecifiedLibGcc;
 }
@@ -1392,12 +1398,12 @@
  ArgStringList &CmdArgs, const ArgList &Args) {
   ToolChain::UnwindLibType UNW = TC.GetUnwindLibType(Args);
   // Targets that don't use unwind libraries.
-  if (TC.getTriple().isAndroid() || TC.getTriple().isOSIAMCU() ||
-  TC.getTriple().isOSBinFormatWasm() ||
+  if ((TC.getTriple().isAndroid() && UNW == ToolChain::UNW_Libgcc) ||
+  TC.getTriple().isOSIAMCU() || TC.getTriple().isOSBinFormatWasm() ||
   UNW == ToolChain::UNW_None)
 return;
 
-  LibGccType LGT = getLibGccType(D, Args);
+  LibGccType LGT = getLibGccType(TC, D, Args);
   bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
   !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
   if (AsNeeded)
@@ -1434,20 +1440,12 @@
 
 static void AddLibgcc(const ToolChain &TC, const Driver &D,
   ArgStringList &CmdArgs, const ArgList &Args) {
-  LibGccType LGT = getLibGccType(D, Args);
+  LibGccType LGT = getLibGccType(TC, D, Args);
   if (LGT != LibGccType::SharedLibGcc)
 CmdArgs.push_back("-lgcc");
   AddUnwindLibrary(TC, D, CmdArgs, Args);
   if (LGT == LibGccType::SharedLibGcc)
 CmdArgs.push_back("-lgcc");
-
-  // According to Android ABI, we have to link with libdl if we are
-  // linking with non-static libgcc.
-  //
-  // NOTE: This fixes a link error on Android MIPS as well.  The non-static
-  // libgcc for MIPS relies on _Unwind_Find_FDE and dl_iterate_phdr from libdl.
-  if (TC.getTriple().isAndroid() && LGT != LibGccType::StaticLibGcc)
-CmdArgs.push_back("-ldl");
 }
 
 void tools::AddRunTimeLibs(const ToolChain &TC, const Driver &D,
@@ -1473,6 +1471,13 @@
   AddLibgcc(TC, D, CmdArgs, Args);
 break;
   }
+
+  // On Android, the unwinder uses dl_iterate_phdr (or one of
+  // dl_unwind_find_exidx/__gnu_Unwind_Find_exidx on arm32) from libdl.so. For
+  // statically-linked executables, these functions come from libc.a instead.
+  if (TC.getTriple().isAndroid() && !Args.hasArg(options::OPT_static) &&
+  !Args.hasArg(options::OPT_static_pie))
+CmdArgs.push_back("-ldl");
 }
 
 SmallString<128> tools::getStatsFileName(const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -929,9 +929,12 @@
 unwindLibType = ToolChain::UNW_None;
   else if (LibName == "platform" || LibName == "") {
 ToolChain::RuntimeLibType RtLibType = GetRuntimeLibType(Args);
-if (RtLibType == ToolChain::RLT_CompilerRT)
-  unwindLibType = ToolChain::UNW_None;
-else if (RtLibType == ToolChain::RLT_Libgcc)
+if (RtLibType == ToolChain::RLT_CompilerRT) {
+  if (getTriple().isAndroid())
+unwindLibType = ToolChain::UNW_CompilerRT;
+  else
+unwindLibType = ToolChain::UNW_None;
+} else if (RtLibType == ToolChain::RLT_Libgcc)
   unwindLibType = ToolChain::UNW_Libgcc;
   } else if (LibName == "libunwind") {
 if (GetRuntimeLibType(Args) == RLT_Libgcc)


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1366,11 +1366,17 @@
 
 enum class LibGccType { UnspecifiedLibGcc, StaticLibGcc, SharedLibGcc };
 
-static LibGccType getLibGccType(const Driver &D, const ArgList &A

[PATCH] D96404: [Android] Default to --rtlib=compiler-rt

2021-03-09 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard updated this revision to Diff 329475.
rprichard added a comment.

Rebase this revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96404

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/Linux.h
  clang/test/Driver/linux-ld.c

Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -288,7 +288,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-ANDROID-NONE %s
 // CHECK-CLANG-ANDROID-NONE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-CLANG-ANDROID-NONE: "-lgcc" "-ldl" "-lc"
+// CHECK-CLANG-ANDROID-NONE: "-l:libunwind.a" "-ldl" "-lc"
 //
 // RUN: %clang -shared -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=aarch64-linux-android -rtlib=platform --unwindlib=platform \
@@ -296,7 +296,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-ANDROID-SHARED %s
 // CHECK-CLANG-ANDROID-SHARED: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-CLANG-ANDROID-SHARED: "-lgcc" "-ldl" "-lc"
+// CHECK-CLANG-ANDROID-SHARED: "-l:libunwind.a" "-ldl" "-lc"
 //
 // RUN: %clang -static -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=aarch64-linux-android -rtlib=platform --unwindlib=platform \
@@ -304,7 +304,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-ANDROID-STATIC %s
 // CHECK-CLANG-ANDROID-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-CLANG-ANDROID-STATIC: "--start-group" "-lgcc" "-lc" "--end-group"
+// CHECK-CLANG-ANDROID-STATIC: "--start-group" "{{[^"]*}}{{/|}}libclang_rt.builtins-aarch64-android.a" "-l:libunwind.a" "-lc" "--end-group"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1  \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform --unwindlib=platform \
@@ -1353,10 +1353,12 @@
 // CHECK-ANDROID: "--enable-new-dtags"
 // CHECK-ANDROID: "{{.*}}{{/|}}crtbegin_dynamic.o"
 // CHECK-ANDROID: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-NOT: "gcc_s"
-// CHECK-ANDROID: "-lgcc"
+// CHECK-ANDROID-NOT: "-lgcc_s"
+// CHECK-ANDROID-NOT: "-lgcc"
+// CHECK-ANDROID: "-l:libunwind.a"
 // CHECK-ANDROID: "-ldl"
-// CHECK-ANDROID-NOT: "gcc_s"
+// CHECK-ANDROID-NOT: "-lgcc_s"
+// CHECK-ANDROID-NOT: "-lgcc"
 // CHECK-ANDROID: "{{.*}}{{/|}}crtend_android.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi -rtlib=platform --unwindlib=platform \
@@ -1409,10 +1411,12 @@
 // CHECK-ANDROID-SO-NOT: "-Bsymbolic"
 // CHECK-ANDROID-SO: "{{.*}}{{/|}}crtbegin_so.o"
 // CHECK-ANDROID-SO: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-SO-NOT: "gcc_s"
-// CHECK-ANDROID-SO: "-lgcc"
+// CHECK-ANDROID-SO-NOT: "-lgcc_s"
+// CHECK-ANDROID-SO-NOT: "-lgcc"
+// CHECK-ANDROID-SO: "-l:libunwind.a"
 // CHECK-ANDROID-SO: "-ldl"
-// CHECK-ANDROID-SO-NOT: "gcc_s"
+// CHECK-ANDROID-SO-NOT: "-lgcc_s"
+// CHECK-ANDROID-SO-NOT: "-lgcc"
 // CHECK-ANDROID-SO: "{{.*}}{{/|}}crtend_so.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi -rtlib=platform --unwindlib=platform \
@@ -1463,10 +1467,12 @@
 // CHECK-ANDROID-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ANDROID-STATIC: "{{.*}}{{/|}}crtbegin_static.o"
 // CHECK-ANDROID-STATIC: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-STATIC-NOT: "gcc_s"
-// CHECK-ANDROID-STATIC: "-lgcc"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc_eh"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc"
+// CHECK-ANDROID-STATIC: "-l:libunwind.a"
 // CHECK-ANDROID-STATIC-NOT: "-ldl"
-// CHECK-ANDROID-STATIC-NOT: "gcc_s"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc_eh"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc"
 // CHECK-ANDROID-STATIC: "{{.*}}{{/|}}crtend_android.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi -rtlib=platform --unwindlib=platform \
@@ -1519,9 +1525,11 @@
 // CHECK-ANDROID-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ANDROID-PIE: "{{.*}}{{/|}}crtbegin_dynamic.o"
 // CHECK-ANDROID-PIE: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-PIE-NOT: "gcc_s"
-// CHECK-ANDROID-PIE: "-lgcc"
-// CHECK-ANDROID-PIE-NOT: "gcc_s"
+// CHECK-ANDROID-PIE-NOT: "-lgcc_s"
+// CHECK-ANDROID-PIE-NOT: "-lgcc"
+// CHECK-ANDROID-PIE: "-l:libunwind.a"
+// CHECK-ANDROID-PIE-NOT: "-lgcc_s"
+// CHECK-ANDROID-PIE-NOT: "-lgcc"
 // CHECK-ANDROID-PIE: "{{.*}}{{/|}}crtend_android.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi \
Index: clang/lib/Driver/ToolChains/Linux.h
===
--- clang/lib/Driver/ToolChains/Linux.h
+++ clang/

[PATCH] D96404: [Android] Default to --rtlib=compiler-rt

2021-03-09 Thread Ryan Prichard via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa478b0a199f4: [Android] Default to --rtlib=compiler-rt 
(authored by rprichard).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96404

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/Linux.h
  clang/test/Driver/linux-ld.c

Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -288,7 +288,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-ANDROID-NONE %s
 // CHECK-CLANG-ANDROID-NONE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-CLANG-ANDROID-NONE: "-lgcc" "-ldl" "-lc"
+// CHECK-CLANG-ANDROID-NONE: "-l:libunwind.a" "-ldl" "-lc"
 //
 // RUN: %clang -shared -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=aarch64-linux-android -rtlib=platform --unwindlib=platform \
@@ -296,7 +296,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-ANDROID-SHARED %s
 // CHECK-CLANG-ANDROID-SHARED: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-CLANG-ANDROID-SHARED: "-lgcc" "-ldl" "-lc"
+// CHECK-CLANG-ANDROID-SHARED: "-l:libunwind.a" "-ldl" "-lc"
 //
 // RUN: %clang -static -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=aarch64-linux-android -rtlib=platform --unwindlib=platform \
@@ -304,7 +304,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-ANDROID-STATIC %s
 // CHECK-CLANG-ANDROID-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-CLANG-ANDROID-STATIC: "--start-group" "-lgcc" "-lc" "--end-group"
+// CHECK-CLANG-ANDROID-STATIC: "--start-group" "{{[^"]*}}{{/|}}libclang_rt.builtins-aarch64-android.a" "-l:libunwind.a" "-lc" "--end-group"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1  \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform --unwindlib=platform \
@@ -1353,10 +1353,12 @@
 // CHECK-ANDROID: "--enable-new-dtags"
 // CHECK-ANDROID: "{{.*}}{{/|}}crtbegin_dynamic.o"
 // CHECK-ANDROID: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-NOT: "gcc_s"
-// CHECK-ANDROID: "-lgcc"
+// CHECK-ANDROID-NOT: "-lgcc_s"
+// CHECK-ANDROID-NOT: "-lgcc"
+// CHECK-ANDROID: "-l:libunwind.a"
 // CHECK-ANDROID: "-ldl"
-// CHECK-ANDROID-NOT: "gcc_s"
+// CHECK-ANDROID-NOT: "-lgcc_s"
+// CHECK-ANDROID-NOT: "-lgcc"
 // CHECK-ANDROID: "{{.*}}{{/|}}crtend_android.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi -rtlib=platform --unwindlib=platform \
@@ -1409,10 +1411,12 @@
 // CHECK-ANDROID-SO-NOT: "-Bsymbolic"
 // CHECK-ANDROID-SO: "{{.*}}{{/|}}crtbegin_so.o"
 // CHECK-ANDROID-SO: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-SO-NOT: "gcc_s"
-// CHECK-ANDROID-SO: "-lgcc"
+// CHECK-ANDROID-SO-NOT: "-lgcc_s"
+// CHECK-ANDROID-SO-NOT: "-lgcc"
+// CHECK-ANDROID-SO: "-l:libunwind.a"
 // CHECK-ANDROID-SO: "-ldl"
-// CHECK-ANDROID-SO-NOT: "gcc_s"
+// CHECK-ANDROID-SO-NOT: "-lgcc_s"
+// CHECK-ANDROID-SO-NOT: "-lgcc"
 // CHECK-ANDROID-SO: "{{.*}}{{/|}}crtend_so.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi -rtlib=platform --unwindlib=platform \
@@ -1463,10 +1467,12 @@
 // CHECK-ANDROID-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ANDROID-STATIC: "{{.*}}{{/|}}crtbegin_static.o"
 // CHECK-ANDROID-STATIC: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-STATIC-NOT: "gcc_s"
-// CHECK-ANDROID-STATIC: "-lgcc"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc_eh"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc"
+// CHECK-ANDROID-STATIC: "-l:libunwind.a"
 // CHECK-ANDROID-STATIC-NOT: "-ldl"
-// CHECK-ANDROID-STATIC-NOT: "gcc_s"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc_eh"
+// CHECK-ANDROID-STATIC-NOT: "-lgcc"
 // CHECK-ANDROID-STATIC: "{{.*}}{{/|}}crtend_android.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi -rtlib=platform --unwindlib=platform \
@@ -1519,9 +1525,11 @@
 // CHECK-ANDROID-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ANDROID-PIE: "{{.*}}{{/|}}crtbegin_dynamic.o"
 // CHECK-ANDROID-PIE: "-L[[SYSROOT]]/usr/lib"
-// CHECK-ANDROID-PIE-NOT: "gcc_s"
-// CHECK-ANDROID-PIE: "-lgcc"
-// CHECK-ANDROID-PIE-NOT: "gcc_s"
+// CHECK-ANDROID-PIE-NOT: "-lgcc_s"
+// CHECK-ANDROID-PIE-NOT: "-lgcc"
+// CHECK-ANDROID-PIE: "-l:libunwind.a"
+// CHECK-ANDROID-PIE-NOT: "-lgcc_s"
+// CHECK-ANDROID-PIE-NOT: "-lgcc"
 // CHECK-ANDROID-PIE: "{{.*}}{{/|}}crtend_android.o"
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi \
Index: clang/lib/Driver/ToolCh

[PATCH] D135142: Use TI.hasBuiltinAtomic() when setting ATOMIC_*_LOCK_FREE values. NFCI

2022-10-04 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard accepted this revision.
rprichard added a comment.
This revision is now accepted and ready to land.

It looks OK to me, but I wonder if you need someone else to accept it also.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135142

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

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

2022-05-26 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

@mgorny Do you mind if I commandeer this Phabricator revision (and also D29542 
, which I would abandon in favor of D59566 
 which is already merged)? I need this CL to 
get libc++ tests passing on i686-linux after D119931 
 was submitted.

I rebased the InitPreprocessor.cpp change and updated two tests 
(Sema/atomic-ops.c and Preprocessor/init-x86.c). The Preprocessor/cuda-types.cu 
test is also failing, but I haven't investigated that yet.

Aside: FreeBSD switched its default CPU from i486 to i686 (D83645 
), but Clang's NetBSD target is still on 486, 
so that doesn't have cmpxchg8b by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D28213

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

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

2022-06-07 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard created this revision.
Herald added subscribers: jsji, kosarev, mattd, gchakrabarti, asavonic, 
kerbowa, pengfei, jvesely.
Herald added a project: All.
rprichard requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, jholewinski.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Previously, NVPTXTargetInfo constructed a TargetInfo using the
TargetOptions::HostTriple field, but did not initialize the target CPU
or CPU features, which resulted in an inaccurate value for
MaxAtomicInlineWidth on x86_32. MaxAtomicInlineWidth is 64 if the host
TargetInfo is initialized with the +cx8 feature (e.g. 586 and up) but
is 32 otherwise.

Instead, add a setAuxTarget override and defer copying from the host
TargetInfo until CompilerInstance::createTarget calls it.

Change setAuxTarget to pass IntrusiveRefCntPtr instead of
a const TargetInfo*. NVPTXTargetInfo needs to retain the host
TargetInfo so that it can check calling conventions.

NVPTXTargetInfo was the only user of the TargetOptions::HostTriple
field, so remove the field.

Also see D29542  and D56318 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127267

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TargetOptions.h
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4472,17 +4472,6 @@
 }
   }
 
-  if (LangOpts.CUDA) {
-// During CUDA device-side compilation, the aux triple is the
-// triple used for host compilation.
-if (LangOpts.CUDAIsDevice)
-  Res.getTargetOpts().HostTriple = Res.getFrontendOpts().AuxTriple;
-  }
-
-  // Set the triple of the host for OpenMP device compile.
-  if (LangOpts.OpenMPIsDevice)
-Res.getTargetOpts().HostTriple = Res.getFrontendOpts().AuxTriple;
-
   ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags, T,
Res.getFrontendOpts().OutputFile, LangOpts);
 
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -118,7 +118,6 @@
   TO->CPU = getFrontendOpts().AuxTargetCPU.getValue();
 if (getFrontendOpts().AuxTargetFeatures)
   TO->FeaturesAsWritten = getFrontendOpts().AuxTargetFeatures.getValue();
-TO->HostTriple = getTarget().getTriple().str();
 setAuxTarget(TargetInfo::CreateTargetInfo(getDiagnostics(), TO));
   }
 
@@ -149,8 +148,8 @@
   // Adjust target options based on codegen options.
   getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
 
-  if (auto *Aux = getAuxTarget())
-getTarget().setAuxTarget(Aux);
+  if (AuxTarget)
+getTarget().setAuxTarget(AuxTarget);
 
   return true;
 }
Index: clang/lib/Basic/Targets/NVPTX.h
===
--- clang/lib/Basic/Targets/NVPTX.h
+++ clang/lib/Basic/Targets/NVPTX.h
@@ -60,12 +60,14 @@
   static const Builtin::Info BuiltinInfo[];
   CudaArch GPU;
   uint32_t PTXVersion;
-  std::unique_ptr HostTarget;
+  IntrusiveRefCntPtr HostTarget;
 
 public:
   NVPTXTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts,
   unsigned TargetPointerWidth);
 
+  void setAuxTarget(IntrusiveRefCntPtr Aux) override;
+
   void getTargetDefines(const LangOptions &Opts,
 MacroBuilder &Builder) const override;
 
Index: clang/lib/Basic/Targets/NVPTX.cpp
===
--- clang/lib/Basic/Targets/NVPTX.cpp
+++ clang/lib/Basic/Targets/NVPTX.cpp
@@ -82,32 +82,28 @@
   else
 resetDataLayout("e-i64:64-i128:128-v16:16-v32:32-n16:32:64");
 
-  // If possible, get a TargetInfo for our host triple, so we can match its
-  // types.
-  llvm::Triple HostTriple(Opts.HostTriple);
-  if (!HostTriple.isNVPTX())
-HostTarget.reset(AllocateTarget(llvm::Triple(Opts.HostTriple), Opts));
-
-  // If no host target, make some guesses about the data layout and return.
-  if (!HostTarget) {
-LongWidth = LongAlign = TargetPointerWidth;
-PointerWidth = PointerAlign = TargetPointerWidth;
-switch (TargetPointerWidth) {
-case 32:
-  SizeType = TargetInfo::UnsignedInt;
-  PtrDiffType = TargetInfo::SignedInt;
-  IntPtrType = TargetInfo::SignedInt;
-  break;
-case 64:
-  SizeType = TargetInfo::UnsignedLong;
-  PtrDiffType = TargetInfo::SignedLong;
-  IntPtrType = TargetInfo::SignedLong;
-  break;
-default:
-  llvm_unreachable("TargetPointer

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

2022-06-07 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard updated this revision to Diff 435037.
rprichard edited the summary of this revision.
rprichard added a comment.

Rebase this revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D28213

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/init-x86.c
  clang/test/Sema/atomic-ops.c


Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -33,11 +33,7 @@
 _Static_assert(__GCC_ATOMIC_INT_LOCK_FREE == __CLANG_ATOMIC_INT_LOCK_FREE, "");
 _Static_assert(__GCC_ATOMIC_LONG_LOCK_FREE == 2, "");
 _Static_assert(__GCC_ATOMIC_LONG_LOCK_FREE == __CLANG_ATOMIC_LONG_LOCK_FREE, 
"");
-#ifdef __i386__
-_Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == 1, "");
-#else
 _Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == 2, "");
-#endif
 _Static_assert(__GCC_ATOMIC_LLONG_LOCK_FREE == __CLANG_ATOMIC_LLONG_LOCK_FREE, 
"");
 _Static_assert(__GCC_ATOMIC_POINTER_LOCK_FREE == 2, "");
 _Static_assert(__GCC_ATOMIC_POINTER_LOCK_FREE == 
__CLANG_ATOMIC_POINTER_LOCK_FREE, "");
Index: clang/test/Preprocessor/init-x86.c
===
--- clang/test/Preprocessor/init-x86.c
+++ clang/test/Preprocessor/init-x86.c
@@ -185,9 +185,9 @@
 // I386:#define __i386__ 1
 // I386:#define i386 1
 
-// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-ALIGN32 %s
-// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-CXX 
-check-prefix I386-LINUX-ALIGN32 %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 -malign-double < /dev/null | 
FileCheck -match-full-lines -check-prefix I386-LINUX -check-prefix 
I386-LINUX-ALIGN64 %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu i486 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-NOCX8 %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-CX8 %s
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=i386-pc-linux-gnu -target-cpu pentium4 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-LINUX -check-prefix I386-LINUX-CX8 
-check-prefix I386-LINUX-CXX %s
 //
 // I386-LINUX-NOT:#define _LP64
 // I386-LINUX:#define __BIGGEST_ALIGNMENT__ 16
@@ -228,8 +228,8 @@
 // I386-LINUX:#define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
 // I386-LINUX:#define __GCC_ATOMIC_CHAR_LOCK_FREE 2
 // I386-LINUX:#define __GCC_ATOMIC_INT_LOCK_FREE 2
-// I386-LINUX-ALIGN32:#define __GCC_ATOMIC_LLONG_LOCK_FREE 1
-// I386-LINUX-ALIGN64:#define __GCC_ATOMIC_LLONG_LOCK_FREE 2
+// I386-LINUX-NOCX8:#define __GCC_ATOMIC_LLONG_LOCK_FREE 1
+// I386-LINUX-CX8:#define __GCC_ATOMIC_LLONG_LOCK_FREE 2
 // I386-LINUX:#define __GCC_ATOMIC_LONG_LOCK_FREE 2
 // I386-LINUX:#define __GCC_ATOMIC_POINTER_LOCK_FREE 2
 // I386-LINUX:#define __GCC_ATOMIC_SHORT_LOCK_FREE 2
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -298,12 +298,12 @@
 
 /// Get the value the ATOMIC_*_LOCK_FREE macro should have for a type with
 /// the specified properties.
-static const char *getLockFreeValue(unsigned TypeWidth, unsigned TypeAlign,
-unsigned InlineWidth) {
+static const char *getLockFreeValue(unsigned TypeWidth, unsigned InlineWidth) {
   // Fully-aligned, power-of-2 sizes no larger than the inline
   // width will be inlined as lock-free operations.
-  if (TypeWidth == TypeAlign && (TypeWidth & (TypeWidth - 1)) == 0 &&
-  TypeWidth <= InlineWidth)
+  // Note: we do not need to check alignment since _Atomic(T) is always
+  // appropriately-aligned in clang.
+  if ((TypeWidth & (TypeWidth - 1)) == 0 && TypeWidth <= InlineWidth)
 return "2"; // "always lock free"
   // We cannot be certain what operations the lib calls might be
   // able to implement as lock-free on future processors.
@@ -1148,7 +1148,6 @@
 #define DEFINE_LOCK_FREE_MACRO(TYPE, Type) 
\
   Builder.defineMacro(Prefix + #TYPE "_LOCK_FREE", 
\
   getLockFreeValue(TI.get##Type##Width(),  
\
-   TI.get##Type##Align(),  
\

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

2022-06-07 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

This change is needed for D28213 , so that the 
`__GCC_ATOMIC_LLONG_LOCK_FREE` macro matches for `-target 
i386-unknown-linux-gnu` between `--cuda-host-only` and `--cuda-device-only`. 
This is tested in clang/test/Preprocessor/cuda-types.cu.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127267

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

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

2022-06-08 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

I don't expect this change to affect the compiler behavior by itself -- is 
there a particular test that should be written?

The code that I'm moving into NVPTXTargetInfo::setAuxTarget is already tested 
via clang/test/Preprocessor/cuda-types.cu.

Aside: There are only two calls to AllocateTarget: one in 
TargetInfo::CreateTargetInfo and one in NVPTXTargetInfo::NVPTXTargetInfo. This 
change removes NVPTXTargetInfo's call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127267

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

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

2022-06-08 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

In D127267#3568388 , @rprichard wrote:

> Aside: There are only two calls to AllocateTarget: one in 
> TargetInfo::CreateTargetInfo and one in NVPTXTargetInfo::NVPTXTargetInfo. 
> This change removes NVPTXTargetInfo's call.

There are various places that call clang::TargetInfo::CreateTargetInfo, though, 
so if a call site had been providing a clang::TargetOptions with a set 
HostTriple field, but not calling clang::TargetInfo::setAuxTarget, then maybe 
the NVPTXTargetInfo behavior could change. I'll audit the calls of 
clang::TargetInfo::CreateTargetInfo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127267

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

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

2022-06-09 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

In D127267#3570269 , @yaxunl wrote:

> This patch is to fix an issue, right? At least we need a test to prevent that 
> issue from happening again.

Yes, this patch is necessary to keep the clang/test/Preprocessor/cuda-types.cu 
test passing after applying D28213 . That test 
is verifying that `__GCC_ATOMIC_LLONG_LOCK_FREE` is the same for 
`--cuda-device-only` and `--cuda-host-only`.

D28213  fixes the value of 
`__GCC_ATOMIC_LLONG_LOCK_FREE` to be `2` when targeting 586 and up, which has 
the cx8 feature (cmpxchg8b). Without this NVPTX patch, the value of 
`__GCC_ATOMIC_LLONG_LOCK_FREE` is `2` for `--cuda-host-only`, but `1` for 
`--cuda-device-only`, because when NVPTXTargetInfo creates the host TargetInfo, 
it only uses the host triple, and doesn't set the target CPU nor initialize CPU 
features. (Specifically, NVPTXTargetInfo calls AllocateTarget but skips all the 
other work that TargetInfo::CreateTargetInfo does, like calling initFeatureMap, 
handleTargetFeatures, and setMaxAtomicWidth.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127267

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

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

2022-06-09 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a subscriber: jlebar.
rprichard added a comment.

Ok, I can upload a patch omitting `*_ATOMIC_LLONG_LOCK_FREE` from the macro 
testing.

FWIW, did you see this comment in clang/lib/Basic/Targets/NVPTX.cpp?:

  // This is a bit of a lie, but it controls __GCC_ATOMIC_XXX_LOCK_FREE, and
  // we need those macros to be identical on host and device, because (among
  // other things) they affect which standard library classes are defined, and
  // we need all classes to be defined on both the host and device.
  MaxAtomicInlineWidth = HostTarget->getMaxAtomicInlineWidth();

It was added in D24407 , apparently so that 
`__GCC_ATOMIC_INT_LOCK_FREE` would be 2 instead of 1. I imagine that's OK, 
because NVPTX does have atomic int?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127267

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

[PATCH] D127465: [CUDA] Ignore __CLANG_ATOMIC_LLONG_LOCK_FREE on i386

2022-06-09 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard created this revision.
Herald added subscribers: mattd, yaxunl.
Herald added a project: All.
rprichard requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The default host CPU for an i386 triple is typically at least an i586,
which has cmpxchg8b (Clang feature, "cx8"). Therefore,
__CLANG_ATOMIC_LLONG_LOCK_FREE is 2 on the host, but the value should
be 1 for the device.

Also, grep for __CLANG_ATOMIC_* instead of __GCC_ATOMIC_*. The CLANG
macros are always emitted, but the GCC macros are omitted for the
*-windows-msvc targets. The __GCC_HAVE_SYNC_COMPARE_AND_SWAP macro
always has GCC in its name, not CLANG, however.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127465

Files:
  clang/test/Preprocessor/cuda-types.cu


Index: clang/test/Preprocessor/cuda-types.cu
===
--- clang/test/Preprocessor/cuda-types.cu
+++ clang/test/Preprocessor/cuda-types.cu
@@ -1,48 +1,53 @@
-// Check that types, widths, __GCC_ATOMIC* macros, etc. match on the host and
+// Check that types, widths, __CLANG_ATOMIC* macros, etc. match on the host and
 // device sides of CUDA compilations.  Note that we filter out long double, as
 // this is intentionally different on host and device.
 //
+// Also ignore __CLANG_ATOMIC_LLONG_LOCK_FREE on i386. The default host CPU for
+// an i386 triple is typically at least an i586, which has cmpxchg8b (Clang
+// feature, "cx8"). Therefore, __CLANG_ATOMIC_LLONG_LOCK_FREE is 2 on the host,
+// but the value should be 1 for the device.
+//
 // FIXME: We really should make __GCC_HAVE_SYNC_COMPARE_AND_SWAP identical on
 // host and device, but architecturally this is difficult at the moment.
 
 // RUN: mkdir -p %t
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
-// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE|_ATOMIC_LLONG_LOCK_FREE' > 
%t/i386-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
-// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE|_ATOMIC_LLONG_LOCK_FREE' > 
%t/i386-device-defines-filtered
 // RUN: diff %t/i386-host-defines-filtered %t/i386-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
 // RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
 // RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
 // RUN: diff %t/x86_64-host-defines-filtered %t/x86_64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target powerpc64-unknown-linux-gnu 
-x cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
 // RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
 // RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
 // RUN: diff %t/powerpc64-host-defines-filtered 
%t/powerpc64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
-// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__CLANG_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE|_ATOMIC_LLONG_LOCK_FREE' > 
%t/i386-msvc-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-windows-msvc -x cuda -E

[PATCH] D127465: [CUDA] Ignore __CLANG_ATOMIC_LLONG_LOCK_FREE on i386

2022-06-09 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added reviewers: tra, yaxunl.
rprichard added a subscriber: mgorny.
rprichard added a comment.

This change is needed to keep the clang/test/Preprocessor/cuda-types.cu test 
passing after D28213  changes the value of 
`__CLANG_ATOMIC_LLONG_LOCK_FREE` from 1 to 2 for i586 and up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127465

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

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

2022-06-09 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

I uploaded D127465 , weakening the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127267

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

[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

2023-09-15 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

In D159292#4646096 , @smeenai wrote:

> Ping.

I asked a couple of other people at Google to take a look at the patches. 
Someone should get to it next week I think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159292

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

[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

2023-08-23 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

> further confirmation if android_get_device_api_level should work from 
> ifunc_resolver

IIRC an ifunc resolver in Bionic can't generally call any functions in libc, 
including `android_get_device_api_level` or `__system_property_get`, because 
the ifunc resolver will typically be called before relocations in libc.so have 
been resolved. I believe an ifunc resolver also can't call 
`__system_property_get` because the libc.so system properties are initialized 
by a constructor function, which isn't called until after relocations are 
applied (`__libc_preinit` -> `__libc_preinit_impl` -> `__libc_init_common` -> 
`__system_properties_init`).

I suspect Bionic ought to apply relocations to libraries in a bottom-up 
fashion, so that libc.so is relocated before the executable or shared objects, 
but I _think_ it's currently top-down. Deferring ifunc relocations until after 
non-ifunc relocations are applied is a separate problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158641

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

[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

2023-08-28 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added inline comments.



Comment at: compiler-rt/lib/builtins/cpu_model.c:1382
+return;
+#if defined(__ANDROID__)
+  // ifunc resolvers don't have hwcaps in arguments on Android API lower

enh wrote:
> srhines wrote:
> > MaskRay wrote:
> > > enh wrote:
> > > > ilinpv wrote:
> > > > > MaskRay wrote:
> > > > > > I am unfamiliar with how Android ndk builds compiler-rt.
> > > > > > 
> > > > > > If `__ANDROID_API__ >= 30`, shall we use the regular Linux code 
> > > > > > path?
> > > > > I think that leads to shipping different compile-rt libraries depend 
> > > > > on ANDROID_API. If this is an option to consider than runtime check 
> > > > > android_get_device_api_level() < 30 can be replaced by 
> > > > > `__ANDROID_API__ < 30`
> > > > depends what you mean... in 10 years or so, yes, no-one is likely to 
> > > > still care about the older API levels and we can just delete this. but 
> > > > until then, no, there's _one_ copy of compiler-rt that everyone uses, 
> > > > and although _OS developers_ don't need to support anything more than a 
> > > > couple of years old, most app developers are targeting far lower API 
> > > > levels than that (to maximize the number of possible customers).
> > > > 
> > > > TL;DR: "you could add that condition to the `#if`, but no-one would use 
> > > > it for a decade". (and i think the comment and `if` below should make 
> > > > it clear enough to future archeologists when this code block can be 
> > > > removed :-) )
> > > My thought was that people build Android with a specific 
> > > `__ANDROID_API__`, and only systems >= this level are supported.
> > > ```
> > > #If __ANDROID_API__ < 30
> > > ...
> > > #endif
> > > ```
> > > 
> > > This code has a greater chance to be removed when it becomes obsoleted. 
> > > The argument is similar to how we find obsoleted GCC workarounds.
> > Yes, the NDK currently just ships the oldest supported target API level for 
> > compiler-rt, while the Android platform build does have access to both the 
> > oldest supported target API level + the most recent target API level, so 
> > that we can make better use of features there.
> > 
> > Maybe I'm missing something, but it's feeling like the NDK users won't be 
> > able to make great use of FMV without us either bumping the minimum level 
> > or shipping multiple runtimes (and then using the #ifdefs properly here).
> > Maybe I'm missing something, but it's feeling like the NDK users won't be 
> > able to make great use of FMV without us either bumping the minimum level 
> > or shipping multiple runtimes (and then using the #ifdefs properly here).
> 
> yeah, that's the point of this code --- it's a runtime check so the NDK "just 
> works".
> 
> but if people want the `__ANDROID_API__` `#if` too, that's fine. (and, like 
> you say, the platform's two variants mean that we'll be testing both code 
> paths, so i'm not worried about "one of these is the only one that anyone's 
> actually building" problem.)
> 
> i have no opinion on whether anyone llvm is more/less likely to understand 
> if/when `if (android_get_device_api_level() < 30)` versus `#if 
> __ANDROID_API__ < 30` can be deleted.
> 
> i think the best argument for leaving this change as-is would be "anyone 
> building their own is less likely to screw up" (since developers struggle all 
> the time with the difference between "target" and "min" api, because the ndk 
> terminology is different to the AndroidManifest.xml stuff that developers are 
> more familiar with, which causes confusion). so if this was in libc++ (where 
> i know people do build their own), i'd argue for the code as-is. but since 
> it's compiler-rt (and i'm not aware that anyone's building that themselves) i 
> don't think it matters either way?
> 
> to be clear, i'm imagining:
> ```
> #if __ANDROID_API__ < 30
>   if (android_get_device_api_level() < 30) {
> setCPUFeature(FEAT_MAX);
> return;
>   }
> #endif
> ```
> (which brings us back to the "this is confusing" --- _just_ having the `#if` 
> would be subtly different, which is why if i'd written this, i'd have written 
> it as-is too!)
Unless I'm missing something, calling android_get_device_api_level doesn't 
work, because (a) the loader hasn't performed the necessary relocations and (b) 
that API reads system properties, which haven't been initialized yet.

Maybe the device API could/should be exported to a /dev file, which is how we 
exported the CPU variant to ifunc resolvers.

We could redesign Bionic so that an ifunc resolver can call 
android_get_device_api_level: e.g:
 - Relocate objects in bottom-up order.
 - Collect ifunc relocations and defer them until after non-ifunc relocations.
 - Have android_get_device_api_level in libc.so call into the loader, which has 
already initialized its copy of the system properties code.

However, with that approach, we can't call android_get_device_api_level unless 
`__ANDROID_API__` is new enough to have the loader e

[PATCH] D159292: [driver] Conditionally include installed libc++ headers for Android

2023-08-31 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

This change looks like an improvement to me. The NDK currently puts the libc++ 
headers into the sysroot, but putting it in the usual toolchain include 
location seems better.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:3108
+SmallString<128> TargetDir(Path);
+llvm::sys::path::append(TargetDir, Target, "c++", Version);
 if (D.getVFS().exists(TargetDir))

Will the `Target` here have an Android API level suffix?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159292

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

[PATCH] D139503: [Headers][ARM] Allow `struct _Unwind_Exception` in unwind.h

2022-12-06 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
rprichard requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Use the same approach as used in libunwind/include/unwind_arm_ehabi.h
(D89570 ) and GCC's unwind-arm-common.h, so 
that _Unwind_Exception can be
used both with and without the struct tag.

Fixes a build failure in libcxxabi/test/forced_unwind1.pass.cpp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139503

Files:
  clang/lib/Headers/unwind.h


Index: clang/lib/Headers/unwind.h
===
--- clang/lib/Headers/unwind.h
+++ clang/lib/Headers/unwind.h
@@ -65,7 +65,8 @@
 #if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) || \
   defined(__ARM_DWARF_EH__) || defined(__SEH__))
 struct _Unwind_Control_Block;
-typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
+typedef struct _Unwind_Control_Block _Unwind_Control_Block;
+#define _Unwind_Exception _Unwind_Control_Block /* Alias */
 #else
 struct _Unwind_Exception;
 typedef struct _Unwind_Exception _Unwind_Exception;


Index: clang/lib/Headers/unwind.h
===
--- clang/lib/Headers/unwind.h
+++ clang/lib/Headers/unwind.h
@@ -65,7 +65,8 @@
 #if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) || \
   defined(__ARM_DWARF_EH__) || defined(__SEH__))
 struct _Unwind_Control_Block;
-typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
+typedef struct _Unwind_Control_Block _Unwind_Control_Block;
+#define _Unwind_Exception _Unwind_Control_Block /* Alias */
 #else
 struct _Unwind_Exception;
 typedef struct _Unwind_Exception _Unwind_Exception;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139503: [Headers][ARM] Allow `struct _Unwind_Exception` in unwind.h

2022-12-06 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

As an aside, it seems interesting that 
libcxxabi/test/forced_unwind[12].pass.cpp don't compile for ARM EHABI when used 
with Clang's unwind.h. AFAICT it's been that way for a long time?

  $ /x/clang14/bin/clang -target arm-linux-gnueabi 
/x/llvm-upstream/llvm-project/libcxxabi/test/forced_unwind1.pass.cpp -c -I 
/x/llvm-upstream/llvm-project/libcxxabi/include
  /x/llvm-upstream/llvm-project/libcxxabi/test/forced_unwind1.pass.cpp:42:42: 
error: typedef '_Unwind_Exception' cannot be referenced with a struct specifier
struct _Unwind_Exception*,
   ^
  
/x/clang+llvm-14.0.0-x86_64-linux-gnu-ubuntu-18.04/lib/clang/14.0.0/include/unwind.h:67:38:
 note: declared here
  typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
   ^
  /x/llvm-upstream/llvm-project/libcxxabi/test/forced_unwind1.pass.cpp:50:49: 
error: typedef '_Unwind_Exception' cannot be referenced with a struct specifier
  static void cleanup(_Unwind_Reason_Code, struct _Unwind_Exception* exc) {
  ^
  
/x/clang+llvm-14.0.0-x86_64-linux-gnu-ubuntu-18.04/lib/clang/14.0.0/include/unwind.h:67:38:
 note: declared here
  typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
   ^
  2 errors generated.

Clang uses its own unwind.h rather than libunwind's unwind.h:

  $ /x/clang14/bin/clang -target arm-linux-gnueabi 
/x/llvm-upstream/llvm-project/libcxxabi/test/forced_unwind1.pass.cpp -c -I 
/x/llvm-upstream/llvm-project/libcxxabi/include -H 2>&1 | grep unwind.h$
  . 
/x/clang+llvm-14.0.0-x86_64-linux-gnu-ubuntu-18.04/lib/clang/14.0.0/include/unwind.h
  $ head -15 
/x/clang+llvm-14.0.0-x86_64-linux-gnu-ubuntu-18.04/lib/clang/14.0.0/include/unwind.h
  /*=== unwind.h - Stack unwinding 
===
   *
   * Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
   * See https://llvm.org/LICENSE.txt for license information.
   * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
   *
   
*===---===
   */
  
  /* See "Data Definitions for libgcc_s" in the Linux Standard Base.*/
  
  #ifndef __CLANG_UNWIND_H
  #define __CLANG_UNWIND_H
  
  #if defined(__APPLE__) && __has_include_next()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139503

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

[PATCH] D139503: [Headers][ARM] Allow `struct _Unwind_Exception` in unwind.h

2022-12-09 Thread Ryan Prichard via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG106a99276f9e: [Headers][ARM] Allow `struct 
_Unwind_Exception` in unwind.h (authored by rprichard).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139503

Files:
  clang/lib/Headers/unwind.h


Index: clang/lib/Headers/unwind.h
===
--- clang/lib/Headers/unwind.h
+++ clang/lib/Headers/unwind.h
@@ -65,7 +65,8 @@
 #if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) || \
   defined(__ARM_DWARF_EH__) || defined(__SEH__))
 struct _Unwind_Control_Block;
-typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
+typedef struct _Unwind_Control_Block _Unwind_Control_Block;
+#define _Unwind_Exception _Unwind_Control_Block /* Alias */
 #else
 struct _Unwind_Exception;
 typedef struct _Unwind_Exception _Unwind_Exception;


Index: clang/lib/Headers/unwind.h
===
--- clang/lib/Headers/unwind.h
+++ clang/lib/Headers/unwind.h
@@ -65,7 +65,8 @@
 #if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) || \
   defined(__ARM_DWARF_EH__) || defined(__SEH__))
 struct _Unwind_Control_Block;
-typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
+typedef struct _Unwind_Control_Block _Unwind_Control_Block;
+#define _Unwind_Exception _Unwind_Control_Block /* Alias */
 #else
 struct _Unwind_Exception;
 typedef struct _Unwind_Exception _Unwind_Exception;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

2021-03-19 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

In D96403#2638883 , @smeenai wrote:

> With NDK r22, I only see libunwind.a for armv7. Will it be provided for other 
> architectures in future NDK versions?

NDK r23 should have a libunwind.a for all architectures. This change isn't in 
the r23 beta 2 that was just released, but it is in the current NDK canary 
build at https://ci.android.com/builds/branches/aosp-master-ndk/grid. (e.g. 
build 7220482 
).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96403

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

[PATCH] D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt

2021-03-22 Thread Ryan Prichard via Phabricator via cfe-commits 
rprichard added a comment.

In D96403#2639210 , @srhines wrote:

> It is actually using `clang-r416183` from toolchains.py 
> .
>  From there, you can go to the manifest for building those prebuilts 
> .
>  This line 
> 
>  shows you the SHA for the `toolchain/llvm-project` used to build the 
> prebuilts. Finally, you can look here 
> 
>  to see what was merged. Note that there's a typo for the revision number in 
> the commit message, but the upstream LLVM SHA 
> 
>  mentioned there is correct. Here 
> 
>  is where this toolchain was built in case that is useful/interesting.

Also: that manifest 

 also links to the `toolchain/llvm_android` git repository 
,
 which has a Python-based build system and a series of patches 

 (mostly cherry-picks) applied to the baseline llvm-project source.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96403

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