[PATCH] D154036: [libc] Add support for creating wrapper headers for offloading in clang

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

The libc specific changes are really minimal straightforward. The GPU side, the 
clang driver changes etc. need a review by a GPU expert.




Comment at: libc/include/CMakeLists.txt:8
+if(LIBC_TARGET_ARCHITECTURE_IS_GPU)
+  include(GetClangResourceDir)
+endif()

Where does this come from?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154036

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

[PATCH] D152226: [FunctionAttrs] Propagate some func/arg/ret attributes from caller to callsite (WIP)

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: libc/CMakeLists.txt:22
+# that clang will use it.
+if(LLVM_LIBC_MAKE_DISCOVERABLE)
+  if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR)

Likely out of date and not required for this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152226

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

In D146973#4225983 , @jdoerfert wrote:

> For most of libc, we might get away with custom GPU headers but eventually it 
> will break "expected to work" user code, at the latest when we arrive at 
> libc++.
> A user can, right now, map a std::vector from the host to the device, and, 
> assuming they properly did the deep copy, it will work.

I do not have any strong opinions about how things ought to work. However, ISTM 
that the above is assuming that the type topology on the host matches the one 
on the GPU. Not sure if that is really an assumption or a restriction or a 
requirement. May be the host / device compatibility ensures this, I do not 
know, I know almost nothing about GPUs. But in general, I would expect a host 
<=> device communication channel to be a curated one. As in, the communication 
model can understand and serialize/deserialize only a small set of primitive 
types and compound types crafted from these primitive types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+  llvm::sys::path::append(P, "llvm-libc");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));

tra wrote:
> sivachandra wrote:
> > jhuber6 wrote:
> > > tra wrote:
> > > > jhuber6 wrote:
> > > > > tra wrote:
> > > > > > Ensuring the right include order will be tricky. Interaction 
> > > > > > between the headers provided by llvm-libc vs the system headers 
> > > > > > will be interesting if we end up mixing the headers. It may be less 
> > > > > > problematic compared to the C++ standard library, but I doubt that 
> > > > > > mixing two implementations would work well here, either. 
> > > > > > 
> > > > > > So, the major question I have -- does adding include path here 
> > > > > > guarantees that we're not picking up the host headers? I do not see 
> > > > > > any changes that would exclude system headers from include paths.
> > > > > > If we do have include paths leading to both llvm-libc and the host 
> > > > > > headers, what's expected to happen if user code ends up including a 
> > > > > > header that's not present in  llvm-libc? Is that possible?
> > > > > > 
> > > > > Right now I'm just kind of relying on an expectation that since this 
> > > > > will be the first `c-isystem` path set, then it will pull in these 
> > > > > libraries first if they exist. It's not captured by the tests, but 
> > > > > compiling with `-v` shows this path being used first in my 
> > > > > experience. So, theoretically, if there is an implementation of said 
> > > > > header in this location, it will be picked up before anything else. 
> > > > > Otherwise it'll just search the other standard locations.
> > > > I think this will be a problem. We're cross-compiling here and for that 
> > > > to work reliably we need to make sure that only target headers are in 
> > > > effect. The problem is that we likely just do not have sufficiently 
> > > > complete set of headers for the GPU. Do we? I have no idea what exactly 
> > > > llvm-libc provides and whether it is sufficient for normal user code to 
> > > > cross-compile for a GPU. 
> > > > 
> > > > It would be interesting to try to compile some C++ code which would 
> > > > include commonly used, but generally target-agnostic, headers like 
> > > >   , etc and check whether we end up pulling 
> > > > in any system headers. Then check what happens if we do not have system 
> > > > headers available at all.
> > > No, it's definitely not complete. Some headers work on the GPU, most 
> > > break in some way or another. The only ones `llvm-libc` provides 
> > > currently is `string.h` and `ctype.h`. But, I figured this wouldn't be a 
> > > problem since it would just go to the system headers anyway if we didn't 
> > > provide them. So we are merely replacing maybe broken with probably works.
> > > 
> > > I was talking with Johannes and he brings up other issues about the idea 
> > > of host-device compatibility between these headers. Since, fundamentally, 
> > > right now `libc` generates its own headers and needs to generate its own 
> > > headers to function. But there can be a problem when migrating data 
> > > between the host and the device is the headers on the host differ 
> > > somewhat to those on the device. I'm not sure what a good overall 
> > > solution to that problem is.
> > Normally, one should not expect target and host headers to be compatible. 
> > So, if you are building for the host, you should use host headers and if 
> > you are building for the target, you should use target headers. Does 
> > general GPU build not follow this approach? May be there are some common 
> > headers but I do not expect them to be from the standard libraries.
> We can generally assume that the GPU and the host do have largely identical 
> types. At least the subset of the types we expect to exchange between host 
> and GPU.
> CUDA compilation cheats, and allows the host to provide most of the headers, 
> with clang and CUDA SDK providing a subset of GPU-side overloads. This way, 
> if GPU-side functions do implicitly rely on the code nominally provided for 
> the host by the host headers, but if we need to code-gen it, we can only do 
> so for a subset that's actually compileable for the GPU -- either constexpr 
> functions, lambdas or `__device__` overloads provided by us.
> 
> Standalone compilation does not have the benefit of having the cake and being 
> able to eat it. It has to be all or nothing, as we do not have the ability to 
> separate the host and GPU code we pull in via headers, nor can we provide a 
> GPU-side overloads. In a way injecting llvm-libc path is a crude attempt to 
> do that by providing GPU-side implementations before we get to include 
> host-side ehaders that would provide the host versions. While it may 
> sometimes work, I do not think it's a robust solution.
> 
> The only sound(-ish) 

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+  llvm::sys::path::append(P, "llvm-libc");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));

jhuber6 wrote:
> tra wrote:
> > jhuber6 wrote:
> > > tra wrote:
> > > > Ensuring the right include order will be tricky. Interaction between 
> > > > the headers provided by llvm-libc vs the system headers will be 
> > > > interesting if we end up mixing the headers. It may be less problematic 
> > > > compared to the C++ standard library, but I doubt that mixing two 
> > > > implementations would work well here, either. 
> > > > 
> > > > So, the major question I have -- does adding include path here 
> > > > guarantees that we're not picking up the host headers? I do not see any 
> > > > changes that would exclude system headers from include paths.
> > > > If we do have include paths leading to both llvm-libc and the host 
> > > > headers, what's expected to happen if user code ends up including a 
> > > > header that's not present in  llvm-libc? Is that possible?
> > > > 
> > > Right now I'm just kind of relying on an expectation that since this will 
> > > be the first `c-isystem` path set, then it will pull in these libraries 
> > > first if they exist. It's not captured by the tests, but compiling with 
> > > `-v` shows this path being used first in my experience. So, 
> > > theoretically, if there is an implementation of said header in this 
> > > location, it will be picked up before anything else. Otherwise it'll just 
> > > search the other standard locations.
> > I think this will be a problem. We're cross-compiling here and for that to 
> > work reliably we need to make sure that only target headers are in effect. 
> > The problem is that we likely just do not have sufficiently complete set of 
> > headers for the GPU. Do we? I have no idea what exactly llvm-libc provides 
> > and whether it is sufficient for normal user code to cross-compile for a 
> > GPU. 
> > 
> > It would be interesting to try to compile some C++ code which would include 
> > commonly used, but generally target-agnostic, headers like  
> >  , etc and check whether we end up pulling in any 
> > system headers. Then check what happens if we do not have system headers 
> > available at all.
> No, it's definitely not complete. Some headers work on the GPU, most break in 
> some way or another. The only ones `llvm-libc` provides currently is 
> `string.h` and `ctype.h`. But, I figured this wouldn't be a problem since it 
> would just go to the system headers anyway if we didn't provide them. So we 
> are merely replacing maybe broken with probably works.
> 
> I was talking with Johannes and he brings up other issues about the idea of 
> host-device compatibility between these headers. Since, fundamentally, right 
> now `libc` generates its own headers and needs to generate its own headers to 
> function. But there can be a problem when migrating data between the host and 
> the device is the headers on the host differ somewhat to those on the device. 
> I'm not sure what a good overall solution to that problem is.
Normally, one should not expect target and host headers to be compatible. So, 
if you are building for the host, you should use host headers and if you are 
building for the target, you should use target headers. Does general GPU build 
not follow this approach? May be there are some common headers but I do not 
expect them to be from the standard libraries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D145584: [libc] Add support for setjmp and longjmp in riscv

[Siva Chandra via Phabricator via cfe-commits]

sivachandra accepted this revision.
sivachandra added inline comments.
This revision is now accepted and ready to land.



Comment at: libc/src/setjmp/riscv64/longjmp.cpp:54-55
+
+  LIBC_INLINE_ASM("seqz %0, %1" : "+r"(buf) : "r"(val) :);
+  LIBC_INLINE_ASM("add %0, %0, %1" : "+r"(buf) : "r"(val), "r"(buf) :);
+}

Your comment is in the previous diff but thanks for the explanation. I think we 
have missed the `val` check for zero in the x86_64 case and should be fixed 
separately.

For the above two instructions, in the interest of reducing the amount of logic 
in inline assembly, can we do:

```
  val = val == 0 ? 1 : val;
  LIBC_INLINE_ASM("add a0, %0, zero\n\t" : : "r"(val) :);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145584

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

[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra accepted this revision.
sivachandra added a comment.
Herald added subscribers: sstefan1, JDevlieghere.

OK for libc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144509

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

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Siva Chandra via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7b28c6cfe04: [clang-tidy][libc] Add an inline function 
checker for the libc project. (authored by sivachandra).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h
  clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s llvmlibc-inline-function-decl %t
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+
+#define LIBC_INLINE inline
+
+namespace __llvm_libc {
+
+LIBC_INLINE int addi(int a, int b) {
+  return a + b;
+}
+
+LIBC_INLINE constexpr long addl(long a, long b) {
+  return a + b;
+}
+
+constexpr long long addll(long long a, long long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addll' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  return a + b;
+}
+
+inline unsigned long addul(unsigned long a, unsigned long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addul' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  return a + b;
+}
+
+class  MyClass {
+  int A;
+public:
+  MyClass() : A(123) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'MyClass' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+  LIBC_INLINE MyClass(int V) : A(V) {}
+
+  constexpr operator int() const { return A; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator int' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+  LIBC_INLINE bool operator==(const MyClass ) {
+return RHS.A == A;
+  }
+
+  static int getVal(const MyClass ) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'getVal' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+return V.A;
+  }
+
+  LIBC_INLINE static void setVal(MyClass , int A) {
+V.A = A;
+  }
+
+  constexpr static int addInt(MyClass , int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'addInt' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+return V.A += A;
+  }
+
+  static LIBC_INLINE int mulInt(MyClass , int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'mulInt' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+return V.A *= A;
+  }
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - llvmlibc-inline-function-decl
+
+llvmlibc-inline-function-decl
+=
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro. See the `libc style guide
+`_ for more information about this macro.
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   For header files without an extension, use an empty string (if there are no
+   other desired extensions) or leave an empty element in the list. E.g.,
+   "h,hh,hpp,hxx," (note the trailing comma).
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -244,6 

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 495265.
sivachandra marked 14 inline comments as done.
sivachandra added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h
  clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s llvmlibc-inline-function-decl %t
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+
+#define LIBC_INLINE inline
+
+namespace __llvm_libc {
+
+LIBC_INLINE int addi(int a, int b) {
+  return a + b;
+}
+
+LIBC_INLINE constexpr long addl(long a, long b) {
+  return a + b;
+}
+
+constexpr long long addll(long long a, long long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addll' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  return a + b;
+}
+
+inline unsigned long addul(unsigned long a, unsigned long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addul' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  return a + b;
+}
+
+class  MyClass {
+  int A;
+public:
+  MyClass() : A(123) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'MyClass' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+  LIBC_INLINE MyClass(int V) : A(V) {}
+
+  constexpr operator int() const { return A; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator int' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+  LIBC_INLINE bool operator==(const MyClass ) {
+return RHS.A == A;
+  }
+
+  static int getVal(const MyClass ) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'getVal' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+return V.A;
+  }
+
+  LIBC_INLINE static void setVal(MyClass , int A) {
+V.A = A;
+  }
+
+  constexpr static int addInt(MyClass , int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'addInt' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+return V.A += A;
+  }
+
+  static LIBC_INLINE int mulInt(MyClass , int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'mulInt' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+return V.A *= A;
+  }
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - llvmlibc-inline-function-decl
+
+llvmlibc-inline-function-decl
+=
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro. See the `libc style guide
+`_ for more information about this macro.
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   For header files without an extension, use an empty string (if there are no
+   other desired extensions) or leave an empty element in the list. E.g.,
+   "h,hh,hpp,hxx," (note the trailing comma).
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -244,6 +244,7 @@
`llvm-twine-local `_, "Yes"

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

Limit the description in the release notes to one sentence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

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

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 492661.
sivachandra marked an inline comment as done.
sivachandra added a comment.

Limit the description in the release notes to one sentence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h
  clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s llvmlibc-inline-function-decl-check %t \
+// RUN:   -- -header-filter=.* -- -I %S
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+
+#define LIBC_INLINE inline
+
+namespace __llvm_libc {
+
+LIBC_INLINE int addi(int a, int b) {
+  return a + b;
+}
+
+LIBC_INLINE constexpr long addl(long a, long b) {
+  return a + b;
+}
+
+constexpr long long addll(long long a, long long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addll' must be tagged with the LIBC_INLINE macro
+  return a + b;
+}
+
+inline unsigned long addul(unsigned long a, unsigned long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addul' must be tagged with the LIBC_INLINE macro
+  return a + b;
+}
+
+class  MyClass {
+  int A;
+public:
+  MyClass() : A(123) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'MyClass' must be tagged with the LIBC_INLINE macro
+
+  LIBC_INLINE MyClass(int V) : A(V) {}
+
+  constexpr operator int() const { return A; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator int' must be tagged with the LIBC_INLINE macro
+
+  LIBC_INLINE bool operator==(const MyClass ) {
+return RHS.A == A;
+  }
+
+  static int getVal(const MyClass ) {
+  // CHECK-MESSAGES: warning: 'getVal' must be tagged with the LIBC_INLINE macro
+return V.A;
+  }
+
+  LIBC_INLINE static void setVal(MyClass , int A) {
+V.A = A;
+  }
+
+  constexpr static int addInt(MyClass , int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'addInt' must be tagged with the LIBC_INLINE macro
+return V.A += A;
+  }
+
+  static LIBC_INLINE int mulInt(MyClass , int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'mulInt' must be tagged with the LIBC_INLINE macro
+return V.A *= A;
+  }
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - llvmlibc-inline-funciton-decl-check
+
+llvmlibc-inline-function-decl-check
+
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro. See the `libc style guide
+`_ for more information about this macro.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -97,6 +97,12 @@
 New checks
 ^^
 
+New `llvmlibc-inline-function-decl-check `_
+check.
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro.
+
 New check aliases
 ^
 
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
@@ -11,6 +11,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "CalleeNamespaceCheck.h"
 #include "ImplementationInNamespaceCheck.h"
+#include "InlineFunctionDeclCheck.h"
 #include "RestrictSystemLibcHeadersCheck.h"
 
 namespace clang::tidy {
@@ -23,6 +24,8 @@
 "llvmlibc-callee-namespace");
 CheckFactories.registerCheck(
 "llvmlibc-implementation-in-namespace");
+CheckFactories.registerCheck(
+"llvmlibc-inline-function-decl-check");
 CheckFactories.registerCheck(
 

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:20
+void InlineFunctionDeclCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(decl(functionDecl()).bind("func_decl"), this);
+}

ClockMan wrote:
> carlosgalvezp wrote:
> > ClockMan wrote:
> > > or maybe even better:
> > > Finder->addMatcher(functionDecl(unless(isExpansionInMainFile()), 
> > > isInline()).bind("func_decl"), this);
> > > Instead of line 26 and 32.
> > I'm not sure that works - if we pass a header directly to clang-tidy, it 
> > will consider it as the "main file", right? That's why the established 
> > pattern is based on `HeaderFileExtensions`, please check out other checks.
> Yes you right, but isInline still can be checked here.
> As for HeaderFileExtensions never used it from both developer and user point 
> of view.
> When running clang-tidy on headers is still better simply create file with 
> single include.
> Maybe if there would be AST_MATCHER for HeaderFileExtensions.
Unfortunately, even `isInline` is not good enough because `isInline` only 
matches functions marked with `inline`: 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchers.h#L7769.
 It misses implicitly inline functions like `constexpr` functions and class 
methods.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:29
+
+  auto SrcBegin = FuncDecl->getSourceRange().getBegin();
+  // Consider functions only in header files.

carlosgalvezp wrote:
> Eugene.Zelenko wrote:
> > Please don't use `auto` unless type is explicitly stated in same statement 
> > or iterator.
> We have a well-established pattern for detecting code in headers, grep for 
> `HeaderFileExtensions` in existing checks.
Thanks! I have now switched over to use the `HeaderFileExtensions` pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

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

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 492597.
sivachandra added a comment.

Update release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h
  clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s llvmlibc-inline-function-decl-check %t \
+// RUN:   -- -header-filter=.* -- -I %S
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+
+#define LIBC_INLINE inline
+
+namespace __llvm_libc {
+
+LIBC_INLINE int addi(int a, int b) {
+  return a + b;
+}
+
+LIBC_INLINE constexpr long addl(long a, long b) {
+  return a + b;
+}
+
+constexpr long long addll(long long a, long long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addll' must be tagged with the LIBC_INLINE macro
+  return a + b;
+}
+
+inline unsigned long addul(unsigned long a, unsigned long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addul' must be tagged with the LIBC_INLINE macro
+  return a + b;
+}
+
+class  MyClass {
+  int A;
+public:
+  MyClass() : A(123) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'MyClass' must be tagged with the LIBC_INLINE macro
+
+  LIBC_INLINE MyClass(int V) : A(V) {}
+
+  constexpr operator int() const { return A; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator int' must be tagged with the LIBC_INLINE macro
+
+  LIBC_INLINE bool operator==(const MyClass ) {
+return RHS.A == A;
+  }
+
+  static int getVal(const MyClass ) {
+  // CHECK-MESSAGES: warning: 'getVal' must be tagged with the LIBC_INLINE macro
+return V.A;
+  }
+
+  LIBC_INLINE static void setVal(MyClass , int A) {
+V.A = A;
+  }
+
+  constexpr static int addInt(MyClass , int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'addInt' must be tagged with the LIBC_INLINE macro
+return V.A += A;
+  }
+
+  static LIBC_INLINE int mulInt(MyClass , int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'mulInt' must be tagged with the LIBC_INLINE macro
+return V.A *= A;
+  }
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - llvmlibc-inline-funciton-decl-check
+
+llvmlibc-inline-function-decl-check
+
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro. See the `libc style guide
+`_ for more information about this macro.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -97,6 +97,13 @@
 New checks
 ^^
 
+New `llvmlibc-inline-function-decl-check `_
+check.
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro. See the `libc style guide
+`_ for more information about this macro.
+
 New check aliases
 ^
 
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
@@ -11,6 +11,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "CalleeNamespaceCheck.h"
 #include "ImplementationInNamespaceCheck.h"
+#include "InlineFunctionDeclCheck.h"
 #include "RestrictSystemLibcHeadersCheck.h"
 
 namespace clang::tidy {
@@ -23,6 +24,8 @@
 "llvmlibc-callee-namespace");
 CheckFactories.registerCheck(
 "llvmlibc-implementation-in-namespace");
+CheckFactories.registerCheck(
+"llvmlibc-inline-function-decl-check");
 

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 492596.
sivachandra added a comment.

Use the HeaderFileExtensions pattern to match constructs in header files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h
  clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
  
clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s llvmlibc-inline-function-decl-check %t \
+// RUN:   -- -header-filter=.* -- -I %S
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+
+#define LIBC_INLINE inline
+
+namespace __llvm_libc {
+
+LIBC_INLINE int addi(int a, int b) {
+  return a + b;
+}
+
+LIBC_INLINE constexpr long addl(long a, long b) {
+  return a + b;
+}
+
+constexpr long long addll(long long a, long long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addll' must be tagged with the LIBC_INLINE macro
+  return a + b;
+}
+
+inline unsigned long addul(unsigned long a, unsigned long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addul' must be tagged with the LIBC_INLINE macro
+  return a + b;
+}
+
+class  MyClass {
+  int A;
+public:
+  MyClass() : A(123) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'MyClass' must be tagged with the LIBC_INLINE macro
+
+  LIBC_INLINE MyClass(int V) : A(V) {}
+
+  constexpr operator int() const { return A; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator int' must be tagged with the LIBC_INLINE macro
+
+  LIBC_INLINE bool operator==(const MyClass ) {
+return RHS.A == A;
+  }
+
+  static int getVal(const MyClass ) {
+  // CHECK-MESSAGES: warning: 'getVal' must be tagged with the LIBC_INLINE macro
+return V.A;
+  }
+
+  LIBC_INLINE static void setVal(MyClass , int A) {
+V.A = A;
+  }
+
+  constexpr static int addInt(MyClass , int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'addInt' must be tagged with the LIBC_INLINE macro
+return V.A += A;
+  }
+
+  static LIBC_INLINE int mulInt(MyClass , int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'mulInt' must be tagged with the LIBC_INLINE macro
+return V.A *= A;
+  }
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - llvmlibc-inline-funciton-decl-check
+
+llvmlibc-inline-function-decl-check
+
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro. See the `libc style guide
+`_ for more information about this macro.
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
@@ -11,6 +11,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "CalleeNamespaceCheck.h"
 #include "ImplementationInNamespaceCheck.h"
+#include "InlineFunctionDeclCheck.h"
 #include "RestrictSystemLibcHeadersCheck.h"
 
 namespace clang::tidy {
@@ -23,6 +24,8 @@
 "llvmlibc-callee-namespace");
 CheckFactories.registerCheck(
 "llvmlibc-implementation-in-namespace");
+CheckFactories.registerCheck(
+"llvmlibc-inline-function-decl-check");
 CheckFactories.registerCheck(
 "llvmlibc-restrict-system-libc-headers");
   }
Index: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h
@@ -0,0 +1,63 @@
+//===-- InlineFunctionDeclCheck.h ---*- C++ -*-===//
+//
+// 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
+//

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 492324.
sivachandra added a comment.

Use explicit type instead of auto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h
  clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
  
clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.cpp
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.h

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.h
@@ -0,0 +1,56 @@
+#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+
+#define LIBC_INLINE inline
+
+namespace __llvm_libc {
+
+LIBC_INLINE int addi(int a, int b) {
+  return a + b;
+}
+
+LIBC_INLINE constexpr long addl(long a, long b) {
+  return a + b;
+}
+
+constexpr long long addll(long long a, long long b) {
+  return a + b;
+}
+
+inline unsigned long addul(unsigned long a, unsigned long b) {
+  return a + b;
+}
+
+class  MyClass {
+  int A;
+public:
+  MyClass() : A(123) {}
+
+  LIBC_INLINE MyClass(int V) : A(V) {}
+
+  constexpr operator int() const { return A; }
+
+  LIBC_INLINE bool operator==(const MyClass ) {
+return RHS.A == A;
+  }
+
+  static int getVal(const MyClass ) {
+return V.A;
+  }
+
+  LIBC_INLINE static void setVal(MyClass , int A) {
+V.A = A;
+  }
+
+  constexpr static int addInt(MyClass , int A) {
+return V.A += A;
+  }
+
+  static LIBC_INLINE int mulInt(MyClass , int A) {
+return V.A *= A;
+  }
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s llvmlibc-inline-function-decl-check %t \
+// RUN:   -- -header-filter=.* -- -I %S
+
+#include "inline-function-decl.h"
+
+namespace __llvm_libc {
+
+// Inline functions in .cpp files need not use the LIBC_INLINE tag.
+inline float addf(float a, float b) {
+  return a + b;
+}
+
+// Non-inline functions should not trigger any warning.
+int mul(int a, int b) {
+  return addf(a * b, b);
+}
+
+// CHECK-MESSAGES: warning: 'addll' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'addul' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'MyClass' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'operator int' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'getVal' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'addInt' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'mulInt' must be tagged with the LIBC_INLINE macro
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - llvmlibc-inline-funciton-decl-check
+
+llvmlibc-inline-function-decl-check
+
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro. See the `libc style guide
+`_ for more information about this macro.
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
@@ -11,6 +11,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "CalleeNamespaceCheck.h"
 #include "ImplementationInNamespaceCheck.h"
+#include "InlineFunctionDeclCheck.h"
 #include "RestrictSystemLibcHeadersCheck.h"
 
 namespace clang::tidy {
@@ -23,6 +24,8 @@
 "llvmlibc-callee-namespace");
 CheckFactories.registerCheck(
 "llvmlibc-implementation-in-namespace");
+CheckFactories.registerCheck(
+"llvmlibc-inline-function-decl-check");
 CheckFactories.registerCheck(
 "llvmlibc-restrict-system-libc-headers");
   }
Index: 

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra created this revision.
sivachandra added a reviewer: aaron.ballman.
Herald added subscribers: ChuanqiXu, carlosgalvezp, ecnelises, tschuett, 
xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
sivachandra requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The new checker checks if inline functions defined in header files are
tagged with the LIBC_INLINE macro. See https://libc.llvm.org/code_style.html
for more information about this macro.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142592

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h
  clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
  
clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.cpp
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.h

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.h
@@ -0,0 +1,56 @@
+#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+
+#define LIBC_INLINE inline
+
+namespace __llvm_libc {
+
+LIBC_INLINE int addi(int a, int b) {
+  return a + b;
+}
+
+LIBC_INLINE constexpr long addl(long a, long b) {
+  return a + b;
+}
+
+constexpr long long addll(long long a, long long b) {
+  return a + b;
+}
+
+inline unsigned long addul(unsigned long a, unsigned long b) {
+  return a + b;
+}
+
+class  MyClass {
+  int A;
+public:
+  MyClass() : A(123) {}
+
+  LIBC_INLINE MyClass(int V) : A(V) {}
+
+  constexpr operator int() const { return A; }
+
+  LIBC_INLINE bool operator==(const MyClass ) {
+return RHS.A == A;
+  }
+
+  static int getVal(const MyClass ) {
+return V.A;
+  }
+
+  LIBC_INLINE static void setVal(MyClass , int A) {
+V.A = A;
+  }
+
+  constexpr static int addInt(MyClass , int A) {
+return V.A += A;
+  }
+
+  static LIBC_INLINE int mulInt(MyClass , int A) {
+return V.A *= A;
+  }
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s llvmlibc-inline-function-decl-check %t \
+// RUN:   -- -header-filter=.* -- -I %S
+
+#include "inline-function-decl.h"
+
+namespace __llvm_libc {
+
+// Inline functions in .cpp files need not use the LIBC_INLINE tag.
+inline float addf(float a, float b) {
+  return a + b;
+}
+
+// Non-inline functions should not trigger any warning.
+int mul(int a, int b) {
+  return addf(a * b, b);
+}
+
+// CHECK-MESSAGES: warning: 'addll' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'addul' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'MyClass' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'operator int' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'getVal' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'addInt' must be tagged with the LIBC_INLINE macro
+// CHECK-MESSAGES: warning: 'mulInt' must be tagged with the LIBC_INLINE macro
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - llvmlibc-inline-funciton-decl-check
+
+llvmlibc-inline-function-decl-check
+
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro. See the `libc style guide
+`_ for more information about this macro.
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
@@ -11,6 +11,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "CalleeNamespaceCheck.h"
 #include "ImplementationInNamespaceCheck.h"
+#include "InlineFunctionDeclCheck.h"
 #include "RestrictSystemLibcHeadersCheck.h"
 
 namespace clang::tidy {
@@ 

[PATCH] D137338: Fix dupe word typos

[Siva Chandra via Phabricator via cfe-commits]

sivachandra accepted this revision.
sivachandra added a comment.

Changes in the libc directory LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137338

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

[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals

[Siva Chandra via Phabricator via cfe-commits]

sivachandra accepted this revision.
sivachandra added a comment.

For libc requirements, LGTM. Please wait for @aaron.ballman for stamping the 
clang-tidy parts.




Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39
+// intercepted.
+static const char *FUNCTIONS_TO_IGNORE_NAMESPACE[] = {
+"__errno_location", "malloc", "calloc", "realloc", "free"};

michaelrj wrote:
> sivachandra wrote:
> > Eugene.Zelenko wrote:
> > > Why not `std::array` of appropriate LLVM container?
> > May be `static const std::uordered_set`? It would likely 
> > make the lookup below much neater.
> an unordered set is a good idea, but the documentation for `StringRef` says 
> it's best not to use them for storage, so I went with `std::string` instead. 
> The code is still a lot nicer.
Literal strings will not count as "storage". They are global data. On the other 
hand, `std::string` will make copies of the literals and require "storage".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113946

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

[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:1
 //===-- CalleeNamespaceCheck.cpp 
--===//
 //

You should add tests for this exception check. See 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:13
 
+#include 
+

Eugene.Zelenko wrote:
> Should  be `` and without newline separation form rest of headers.
Looks like this is present only for the debug `printf`?



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39
+// intercepted.
+static const char *FUNCTIONS_TO_IGNORE_NAMESPACE[] = {
+"__errno_location", "malloc", "calloc", "realloc", "free"};

Eugene.Zelenko wrote:
> Why not `std::array` of appropriate LLVM container?
May be `static const std::uordered_set`? It would likely make 
the lookup below much neater.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:59
+llvm::StringRef(FUNCTIONS_TO_IGNORE_NAMESPACE[i]))) {
+  printf("String found %s\n", FuncDecl->getName().str().c_str());
+  return;

lntue wrote:
> Look like diag() is used to print messages in this module?
This looks like a debug `printf`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113946

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

[PATCH] D104854: Introduce intrinsic llvm.isnan

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22193
+DAG.getConstant(0x45, DL, MVT::i8));
+
+  return DAG.getSetCC(DL, ResultVT, Extract, DAG.getConstant(1, DL, MVT::i8),

While I do not understand the code mechanics of this patch, I am mostly in 
agreement with the general direction of this patch. However, it has lead to a 
change in behavior wrt 80-bit x86 floating point numbers. Unlike the 32-bit and 
64-bit floating point numbers, 80-bit numbers have an additional class of 
"Unsupported Numbers". Those numbers were previously treated as NaNs. Since 
this change uses the `fxam` instruction to classify the input number, that is 
not the case any more as the `fxam` instruction distinguishes between 
unsupported numbers and NaNs. So, to restore the previous behavior, can we 
extend this patch to treat unsupported numbers as NaNs? At a high level, what I 
am effectively saying is that we should implement `isnan` this way:

```
bool isnan(long double x) {
  uint16_t status;
  __asm__ __volatile__("fldt %0" : : "m"(x));
  __asm__ __volatile__("fxam");
  __asm__ __volatile__("fnstsw %0": "=m"(status):);
  uint16_t c0c2c3 = (status >> 8) & 0x45;
  return c0c2c3 <= 1; // This patch seems to be only doing c0c2c3 == 1 check.
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104854

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

In D97736#2605535 , @phosek wrote:

> Have you considered using an input linker script? We could generate `libc.so` 
> that could look something like:
>
>   INPUT(libllvmlibc.a /lib/libc.so)
>
> We would need to pass `--sysroot` to the linker for this to work. The driver 
> could remain completely agnostic of whether you're using LLVM libc or not.

Yes, that was also considered. Those downstream users who have the flexibility 
to do it that way should be able to do it that way. However, not all downstream 
users or normal clang users will have that liberty [1]. Another point to note 
is that we will have to do this with all libc components like `libc.so`, 
`libm.so` etc.

[1] I think all of this can be done. For example, we can set all this up when 
building a distribution. However, I am not sure this is worth it when we know 
this is a transient phase. Soon, when LLVM libc is complete enough, a more 
appropriate option would be the one which allows choosing a libc as Eric 
pointed out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 328380.
sivachandra added a comment.

Remove empty '//' in lit test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  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
@@ -232,7 +232,27 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 
'static-pie'
-//
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lm"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lc"
+
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -670,6 +670,23 @@
 
   Args.AddAllArgs(CmdArgs, options::OPT_T);
 
+  if (Args.hasArg(options::OPT_experimental_link_llvmlibc)) {
+// We should add -lllvmlibc after all other link args have been 
accumulated.
+// This way, we can iterate over all the link args and add -lllvmlibc 
before
+// each libc library. This will also ensure that we prepend -lllvmlibc
+// before any used listed -lm options.
+ArgStringList WithLLVMLibc;
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread
+// functions available.
+WithLLVMLibc.push_back("-lllvmlibc");
+  }
+  WithLLVMLibc.push_back(Arg.data());
+}
+CmdArgs = WithLLVMLibc;
+  }
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3623,6 +3623,7 @@
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group, 
Flags<[NoArgumentUnused]>;


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -232,7 +232,27 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 'static-pie'
-//
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: 

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

In D97736#2605432 , @echristo wrote:

> In addition to the bikeshed below, I'm not a huge fan of this in general. I 
> think we should assume that the libc we link is complete and thus it would 
> just be named "libc." and in a sysroot somewhere. Another option is 
> perhaps looking at the rtlib option, but I'd want to see a bit of a writeup 
> there so we can see what we'd be doing.

I missed responding to the "writeup" part earlier. But, I am not sure what I 
should be writing about. I feel that there is some confusion that this option 
can be used to switch between libcs. As I said in the earlier reply, the idea 
is not to provide a way to switch between libcs. Rather, the goal is to provide 
a way to use LLVM libc along with the system libc with the effect that LLVM 
libc symbols will be preferred by the linker over the system libc symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

In D97736#2605432 , @echristo wrote:

> In addition to the bikeshed below, I'm not a huge fan of this in general. I 
> think we should assume that the libc we link is complete and thus it would 
> just be named "libc." and in a sysroot somewhere. Another option is 
> perhaps looking at the rtlib option, but I'd want to see a bit of a writeup 
> there so we can see what we'd be doing.

By not giving it a `libc.` name, we are actually implying that, "it is not 
the full libc that you expect." It is expected that it will be used along with 
another full libc. When LLVM libc is complete enough, then yes we should be 
giving it the conventional name of `libc.` and the experimental option can 
be thrown out.

In D97736#2605159 , @MaskRay wrote:

> If the end goal is to provide a complete libc, and currently the usage is 
> expected to shadow system libc (this has to be very careful, as I don't know 
> how shadowing some important components like pthread shall work), perhaps the 
> name should convey this point?

Yes. So, we have not done it yet, but the plan is to be able to build LLVM in 
two different modes:

1. The default mode: This mode only builds and packages the ABI independent and 
ABI agnostic parts of LLVM libc. The binaries produced from this mode are to be 
used with the `-experimental-llvm-libc`. The plan is also to make default mode 
work under `LLVM_RUNTIMES_BUILD`.
2. The full libc mode: In this mode, LLVM libc will be built as if it is a full 
libc. A special CMake option needs to be set to build in this mode. The 
binaries produced in this mode will include everything, including ABI sensitive 
pieces. They will be tested on bots but will not be practically useful for many 
months as we are still building out the libc.




Comment at: clang/include/clang/Driver/Options.td:3626
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;

echristo wrote:
> Bikeshedding: Not a huge fan of this name. I'd rather a more general option 
> if we feel the need to select libc from an option rather than a sysroot. 
The idea is not to provide an option to "choose" a libc. The idea is that this 
option will help one use what LLVM libc provides while getting the rest from 
the system libc. I would think that the option to "choose" a libc is not 
related to this as LLVM libc is not complete.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:680
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread

sivachandra wrote:
> phosek wrote:
> > This wouldn't handle the case where you use `-nolibc path/to/libc.a` in 
> > which case you'd have to manually pass in the `libllvmllibc.a`, but I'm not 
> > sure if that's a case we care about.
> Yeah, its hard to cater to all combinations.  In this case though, assuming 
> `libllvmlibc.a` is available in path, one can add `-lllvmlibc` in the right 
> place. 
Another point to note is that `-nolibc` does not prevent `-lm` from getting 
added for C++ compilations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:680
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread

phosek wrote:
> This wouldn't handle the case where you use `-nolibc path/to/libc.a` in which 
> case you'd have to manually pass in the `libllvmllibc.a`, but I'm not sure if 
> that's a case we care about.
Yeah, its hard to cater to all combinations.  In this case though, assuming 
`libllvmlibc.a` is available in path, one can add `-lllvmlibc` in the right 
place. 



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:684
+WithLLVMLibc.push_back("-lllvmlibc");
+WithLLVMLibc.push_back(Arg.data());
+  } else {

phosek wrote:
> You can move this after the condition and omit the `else` branch.
[Shame cube] Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

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

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 328340.
sivachandra added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97736

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  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
@@ -233,6 +233,26 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 
'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+//
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lm"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lc"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -670,6 +670,23 @@
 
   Args.AddAllArgs(CmdArgs, options::OPT_T);
 
+  if (Args.hasArg(options::OPT_experimental_link_llvmlibc)) {
+// We should add -lllvmlibc after all other link args have been 
accumulated.
+// This way, we can iterate over all the link args and add -lllvmlibc 
before
+// each libc library. This will also ensure that we prepend -lllvmlibc
+// before any used listed -lm options.
+ArgStringList WithLLVMLibc;
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread
+// functions available.
+WithLLVMLibc.push_back("-lllvmlibc");
+  }
+  WithLLVMLibc.push_back(Arg.data());
+}
+CmdArgs = WithLLVMLibc;
+  }
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3623,6 +3623,7 @@
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group, 
Flags<[NoArgumentUnused]>;


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -233,6 +233,26 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+//
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// 

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra created this revision.
sivachandra added a reviewer: phosek.
Herald added subscribers: jansvoboda11, dang.
sivachandra requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The experimental option is named -experimental-link-llvmlibc. Specifying
it will put -lllvmlibc before each system libc library component on the
link command line. This way, even if the user specifies -lm, functions
from LLVM libc will be picked instead of functions from the system libc
math library. Also, sanitizer and other options add -lm at few other
places. By putting -lllvmlibc right before each of them, we ensure that
functions available in LLVM libc are picked up by the linker.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97736

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  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
@@ -233,6 +233,26 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 
'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC: "-lllvmlibc" "-lc"
+//
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o 
%t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN: -lm \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-LINK-LLVMLIBC-LM %s
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lm"
+// CHECK-CLANG-LD-LINK-LLVMLIBC-LM: "-lllvmlibc" "-lc"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -670,6 +670,25 @@
 
   Args.AddAllArgs(CmdArgs, options::OPT_T);
 
+  if (Args.hasArg(options::OPT_experimental_link_llvmlibc)) {
+// We should add -lllvmlibc after all other link args have been 
accumulated.
+// This way, we can iterate over all the link args and add -lllvmlibc 
before
+// each libc library. This will also ensure that we prepend -lllvmlibc
+// before any used listed -lm options.
+ArgStringList WithLLVMLibc;
+for (StringRef Arg : CmdArgs) {
+  if (Arg == "-lm" || Arg == "-lc") {
+// TODO: Add -lllvmlibc before -lpthread when LLVM libc has pthread
+// functions available.
+WithLLVMLibc.push_back("-lllvmlibc");
+WithLLVMLibc.push_back(Arg.data());
+  } else {
+WithLLVMLibc.push_back(Arg.data());
+  }
+}
+CmdArgs = WithLLVMLibc;
+  }
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3623,6 +3623,7 @@
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def experimental_link_llvmlibc : Flag<["-"], "experimental-link-llvmlibc">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group, 
Flags<[NoArgumentUnused]>;


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -233,6 +233,26 @@
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
 // CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 'static-pie'
 //
+// RUN: %clang -experimental-link-llvmlibc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: 

[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp:16
+
+namespace namespaceG {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: '__llvm_libc' needs to be the 
outermost namespace.

Can you add `__llvm_libc` as a nested namespace here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76818



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

[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp:30
+// Wrapped in correct namespace.
+namespace __llvm_libc {
+class ClassB;

For completeness, can you include a nested namespace and a function declaration 
here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76818



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

[PATCH] D76744: [clang-tidy] Add check to ensure llvm-libc implementations are defined in correct namespace.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

Instead of this narrow check, what we really want is a check to ensure that all 
implementation detail resides in the namespace `__llvm_libc`. Anything outside 
would be special and requiring a `NOLINT...` for them is reasonable. Have you 
considered such an approach?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76744



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

[PATCH] D70416: [Driver] Make -static-libgcc imply static libunwind

[Siva Chandra via Phabricator via cfe-commits]

sivachandra accepted this revision.
sivachandra added a comment.
This revision is now accepted and ready to land.

Please wait for saugustine's acceptance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70416



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

[PATCH] D62606: [Driver] -static-pie: add -z text

[Siva Chandra via Phabricator via cfe-commits]

sivachandra accepted this revision.
sivachandra added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D62606



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

[PATCH] D59841: [Gnu Driver] Let -static-pie win if it is specified along with -pie or -static.

[Siva Chandra via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361312: Let -static-pie win if it is specified along with 
-pie or -static. (authored by sivachandra, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59841?vs=200577=200584#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59841

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/linux-ld.c

Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -311,7 +311,7 @@
 
 static bool getPIE(const ArgList , const toolchains::Linux ) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
@@ -321,6 +321,26 @@
   return A->getOption().matches(options::OPT_pie);
 }
 
+static bool getStaticPIE(const ArgList ,
+ const toolchains::Linux ) {
+  bool HasStaticPIE = Args.hasArg(options::OPT_static_pie);
+  // -no-pie is an alias for -nopie. So, handling -nopie takes care of
+  // -no-pie as well.
+  if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
+const Driver  = ToolChain.getDriver();
+const llvm::opt::OptTable  = D.getOpts();
+const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
+D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
+  }
+  return HasStaticPIE;
+}
+
+static bool getStatic(const ArgList ) {
+  return Args.hasArg(options::OPT_static) &&
+  !Args.hasArg(options::OPT_static_pie);
+}
+
 void tools::gnutools::Linker::ConstructJob(Compilation , const JobAction ,
const InputInfo ,
const InputInfoList ,
@@ -336,7 +356,8 @@
   const bool isAndroid = ToolChain.getTriple().isAndroid();
   const bool IsIAMCU = ToolChain.getTriple().isOSIAMCU();
   const bool IsPIE = getPIE(Args, ToolChain);
-  const bool IsStaticPIE = Args.hasArg(options::OPT_static_pie);
+  const bool IsStaticPIE = getStaticPIE(Args, ToolChain);
+  const bool IsStatic = getStatic(Args);
   const bool HasCRTBeginEndFiles =
   ToolChain.getTriple().hasEnvironment() ||
   (ToolChain.getTriple().getVendor() != llvm::Triple::MipsTechnologies);
@@ -408,7 +429,7 @@
 return;
   }
 
-  if (Args.hasArg(options::OPT_static)) {
+  if (IsStatic) {
 if (Arch == llvm::Triple::arm || Arch == llvm::Triple::armeb ||
 Arch == llvm::Triple::thumb || Arch == llvm::Triple::thumbeb)
   CmdArgs.push_back("-Bstatic");
@@ -418,7 +439,7 @@
 CmdArgs.push_back("-shared");
   }
 
-  if (!Args.hasArg(options::OPT_static)) {
+  if (!IsStatic) {
 if (Args.hasArg(options::OPT_rdynamic))
   CmdArgs.push_back("-export-dynamic");
 
@@ -465,7 +486,7 @@
   }
   if (P.empty()) {
 const char *crtbegin;
-if (Args.hasArg(options::OPT_static))
+if (IsStatic)
   crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
 else if (Args.hasArg(options::OPT_shared))
   crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
@@ -520,7 +541,7 @@
 
   if (!Args.hasArg(options::OPT_nostdlib)) {
 if (!Args.hasArg(options::OPT_nodefaultlibs)) {
-  if (Args.hasArg(options::OPT_static) || IsStaticPIE)
+  if (IsStatic || IsStaticPIE)
 CmdArgs.push_back("--start-group");
 
   if (NeedsSanitizerDeps)
@@ -556,7 +577,7 @@
   if (IsIAMCU)
 CmdArgs.push_back("-lgloss");
 
-  if (Args.hasArg(options::OPT_static) || IsStaticPIE)
+  if (IsStatic || IsStaticPIE)
 CmdArgs.push_back("--end-group");
   else
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -458,4 +458,5 @@
   "command line to use the libc++ standard library instead">,
   InGroup>;
 
+def err_drv_cannot_mix_options : Error<"cannot specify '%1' along with '%0'">;
 }
Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -194,6 +194,39 @@
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
 // CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
+// RUN: %clang -static-pie -pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: 

[PATCH] D59841: [Gnu Driver] Let -static-pie win if it is specified along with -pie or -static.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra marked an inline comment as done.
sivachandra added a comment.

PTAL




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:311
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;

saugustine wrote:
> It's not clear to me that the command line -static-pie -no-pie should result 
> in static-pie, given the way the rest of that function is coded.
> 
> I might make it a ternary enum: "nothing" "pie" "static-pie" with the last 
> one winning. That method seems more consistent with current behavior.
> 
> This would allow anyone checking the state of pie to use a single function 
> and just check the result.
static-pie is now handled in its own separate function below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59841



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

[PATCH] D59841: [Gnu Driver] Let -static-pie win if it is specified along with -pie or -static.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra updated this revision to Diff 200577.
sivachandra added a comment.

Let -static-pie win if specified along with -pie or -static.

Also, treat specifying -nopie/-no-pie along with -static-pie as an error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59841

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  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
@@ -194,6 +194,39 @@
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
 // CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
+// RUN: %clang -static-pie -pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-PIE %s
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "-static"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "-pie"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "--no-dynamic-linker"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+//
+// RUN: %clang -static-pie -static -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-STATIC %s
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "-static"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "-pie"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "--no-dynamic-linker"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+//
+// RUN: %clang -static-pie -nopie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE-NOPIE %s
+// CHECK-CLANG-LD-STATIC-PIE-NOPIE: error: cannot specify 'nopie' along with 'static-pie'
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -311,7 +311,7 @@
 
 static bool getPIE(const ArgList , const toolchains::Linux ) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
@@ -321,6 +321,26 @@
   return A->getOption().matches(options::OPT_pie);
 }
 
+static bool getStaticPIE(const ArgList ,
+ const toolchains::Linux ) {
+  bool HasStaticPIE = Args.hasArg(options::OPT_static_pie);
+  // -no-pie is an alias for -nopie. So, handling -nopie takes care of
+  // -no-pie as well.
+  if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
+const Driver  = ToolChain.getDriver();
+const llvm::opt::OptTable  = D.getOpts();
+const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
+D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
+  }
+  return HasStaticPIE;
+}
+
+static bool getStatic(const ArgList ) {
+  return Args.hasArg(options::OPT_static) &&
+  !Args.hasArg(options::OPT_static_pie);
+}
+
 void tools::gnutools::Linker::ConstructJob(Compilation , const JobAction ,
const InputInfo ,
const InputInfoList ,
@@ -336,7 +356,8 @@
   const bool isAndroid = ToolChain.getTriple().isAndroid();
   const bool IsIAMCU = ToolChain.getTriple().isOSIAMCU();
   const bool IsPIE = getPIE(Args, ToolChain);
-  const bool IsStaticPIE = Args.hasArg(options::OPT_static_pie);
+  const bool IsStaticPIE = getStaticPIE(Args, ToolChain);
+  const bool IsStatic = getStatic(Args);
   const bool HasCRTBeginEndFiles =
   ToolChain.getTriple().hasEnvironment() ||
   (ToolChain.getTriple().getVendor() != 

[PATCH] D59841: [Gnu Driver] Is -pie and -static-pie are both passed, let -static-pie win.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra created this revision.
sivachandra added a reviewer: saugustine.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

A static-pie binary is a PIE after all, so letting -static-pie win makes
the resulting binary statically linked as well as being a PIE.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59841

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  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
@@ -189,6 +189,19 @@
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
 // CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" 
"--end-group"
 //
+// RUN: %clang -pie -static-pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-PIE-STATIC-PIE %s
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-static"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-pie"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--no-dynamic-linker"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" 
"--end-group"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -308,7 +308,7 @@
 
 static bool getPIE(const ArgList , const toolchains::Linux ) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -189,6 +189,19 @@
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
 // CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
+// RUN: %clang -pie -static-pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-PIE-STATIC-PIE %s
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-static"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-pie"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--no-dynamic-linker"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -308,7 +308,7 @@
 
 static bool getPIE(const ArgList , const toolchains::Linux ) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58307: [Clang Driver] Add support for "-static-pie" argument to the Clang driver.

[Siva Chandra via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354502: [Clang Driver] Add support for 
-static-pie argument to the Clang driver. (authored by sivachandra, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58307?vs=187091=187628#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58307

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/test/Driver/linux-ld.c

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2508,6 +2508,7 @@
 def no_pthread : Flag<["-"], "no-pthread">, Flags<[CC1Option]>;
 def p : Flag<["-"], "p">;
 def pie : Flag<["-"], "pie">;
+def static_pie : Flag<["-"], "static-pie">;
 def read__only__relocs : Separate<["-"], "read_only_relocs">;
 def remap : Flag<["-"], "remap">;
 def rewrite_objc : Flag<["-"], "rewrite-objc">, Flags<[DriverOption,CC1Option]>,
Index: cfe/trunk/test/Driver/linux-ld.c
===
--- cfe/trunk/test/Driver/linux-ld.c
+++ cfe/trunk/test/Driver/linux-ld.c
@@ -176,6 +176,19 @@
 // CHECK-CLANG-NO-LIBGCC-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-CLANG-NO-LIBGCC-STATIC: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
+// RUN: %clang -static-pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE %s
+// CHECK-CLANG-LD-STATIC-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-STATIC-PIE: "-static"
+// CHECK-CLANG-LD-STATIC-PIE: "-pie"
+// CHECK-CLANG-LD-STATIC-PIE: "--no-dynamic-linker"
+// CHECK-CLANG-LD-STATIC-PIE: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -333,6 +333,7 @@
   const bool isAndroid = ToolChain.getTriple().isAndroid();
   const bool IsIAMCU = ToolChain.getTriple().isOSIAMCU();
   const bool IsPIE = getPIE(Args, ToolChain);
+  const bool IsStaticPIE = Args.hasArg(options::OPT_static_pie);
   const bool HasCRTBeginEndFiles =
   ToolChain.getTriple().hasEnvironment() ||
   (ToolChain.getTriple().getVendor() != llvm::Triple::MipsTechnologies);
@@ -353,6 +354,12 @@
   if (IsPIE)
 CmdArgs.push_back("-pie");
 
+  if (IsStaticPIE) {
+CmdArgs.push_back("-static");
+CmdArgs.push_back("-pie");
+CmdArgs.push_back("--no-dynamic-linker");
+  }
+
   if (Args.hasArg(options::OPT_rdynamic))
 CmdArgs.push_back("-export-dynamic");
 
@@ -402,7 +409,7 @@
 if (Args.hasArg(options::OPT_rdynamic))
   CmdArgs.push_back("-export-dynamic");
 
-if (!Args.hasArg(options::OPT_shared)) {
+if (!Args.hasArg(options::OPT_shared) && !IsStaticPIE) {
   const std::string Loader =
   D.DyldPrefix + ToolChain.getDynamicLinker(Args);
   CmdArgs.push_back("-dynamic-linker");
@@ -421,6 +428,8 @@
   crt1 = "gcrt1.o";
 else if (IsPIE)
   crt1 = "Scrt1.o";
+else if (IsStaticPIE)
+  crt1 = "rcrt1.o";
 else
   crt1 = "crt1.o";
   }
@@ -438,14 +447,14 @@
 crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
   else if (Args.hasArg(options::OPT_shared))
 crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
-  else if (IsPIE)
+  else if (IsPIE || IsStaticPIE)
 crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
   else
 crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
 
   if (HasCRTBeginEndFiles)
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtbegin)));
-}
+	}
 
 // Add crtfastmath.o if available and fast math is enabled.
 ToolChain.AddFastMathRuntimeIfAvailable(Args, CmdArgs);
@@ -489,7 +498,7 @@
 
   if (!Args.hasArg(options::OPT_nostdlib)) {
 if (!Args.hasArg(options::OPT_nodefaultlibs)) {
-  if (Args.hasArg(options::OPT_static))
+  if (Args.hasArg(options::OPT_static) || IsStaticPIE)
 CmdArgs.push_back("--start-group");
 
   if (NeedsSanitizerDeps)
@@ -525,7 +534,7 @@
   if (IsIAMCU)

[PATCH] D58307: [Clang Driver] Add support for "-static-pie" argument to the Clang driver.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change mimics GCC's support for the "-static-pie" argument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58307

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  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
@@ -176,6 +176,19 @@
 // CHECK-CLANG-NO-LIBGCC-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-CLANG-NO-LIBGCC-STATIC: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
+// RUN: %clang -static-pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE %s
+// CHECK-CLANG-LD-STATIC-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-STATIC-PIE: "-static"
+// CHECK-CLANG-LD-STATIC-PIE: "-pie"
+// CHECK-CLANG-LD-STATIC-PIE: "--no-dynamic-linker"
+// CHECK-CLANG-LD-STATIC-PIE: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -333,6 +333,7 @@
   const bool isAndroid = ToolChain.getTriple().isAndroid();
   const bool IsIAMCU = ToolChain.getTriple().isOSIAMCU();
   const bool IsPIE = getPIE(Args, ToolChain);
+  const bool IsStaticPIE = Args.hasArg(options::OPT_static_pie);
   const bool HasCRTBeginEndFiles =
   ToolChain.getTriple().hasEnvironment() ||
   (ToolChain.getTriple().getVendor() != llvm::Triple::MipsTechnologies);
@@ -353,6 +354,12 @@
   if (IsPIE)
 CmdArgs.push_back("-pie");
 
+  if (IsStaticPIE) {
+CmdArgs.push_back("-static");
+CmdArgs.push_back("-pie");
+CmdArgs.push_back("--no-dynamic-linker");
+  }
+
   if (Args.hasArg(options::OPT_rdynamic))
 CmdArgs.push_back("-export-dynamic");
 
@@ -402,7 +409,7 @@
 if (Args.hasArg(options::OPT_rdynamic))
   CmdArgs.push_back("-export-dynamic");
 
-if (!Args.hasArg(options::OPT_shared)) {
+if (!Args.hasArg(options::OPT_shared) && !IsStaticPIE) {
   const std::string Loader =
   D.DyldPrefix + ToolChain.getDynamicLinker(Args);
   CmdArgs.push_back("-dynamic-linker");
@@ -421,6 +428,8 @@
   crt1 = "gcrt1.o";
 else if (IsPIE)
   crt1 = "Scrt1.o";
+else if (IsStaticPIE)
+  crt1 = "rcrt1.o";
 else
   crt1 = "crt1.o";
   }
@@ -438,14 +447,14 @@
 crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
   else if (Args.hasArg(options::OPT_shared))
 crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
-  else if (IsPIE)
+  else if (IsPIE || IsStaticPIE)
 crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
   else
 crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
 
   if (HasCRTBeginEndFiles)
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtbegin)));
-}
+	}
 
 // Add crtfastmath.o if available and fast math is enabled.
 ToolChain.AddFastMathRuntimeIfAvailable(Args, CmdArgs);
@@ -489,7 +498,7 @@
 
   if (!Args.hasArg(options::OPT_nostdlib)) {
 if (!Args.hasArg(options::OPT_nodefaultlibs)) {
-  if (Args.hasArg(options::OPT_static))
+  if (Args.hasArg(options::OPT_static) || IsStaticPIE)
 CmdArgs.push_back("--start-group");
 
   if (NeedsSanitizerDeps)
@@ -524,7 +533,7 @@
   if (IsIAMCU)
 CmdArgs.push_back("-lgloss");
 
-  if (Args.hasArg(options::OPT_static))
+  if (Args.hasArg(options::OPT_static) || IsStaticPIE)
 CmdArgs.push_back("--end-group");
   else
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
@@ -541,7 +550,7 @@
   const char *crtend;
   if (Args.hasArg(options::OPT_shared))
 crtend = isAndroid ? "crtend_so.o" : "crtendS.o";
-  else if (IsPIE)
+  else if (IsPIE || IsStaticPIE)
 crtend = isAndroid ? "crtend_android.o" : "crtendS.o";
   else
 crtend = isAndroid ? "crtend_android.o" : "crtend.o";
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1137,19 +1137,22 @@
   bool isCygMing =