[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 204187.
plotfi marked 11 inline comments as done.
plotfi added a comment.

addressing @rupprecht's feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// CHECK-TAPI-NOT: _ZNK1Q5func1Ev
+// CHECK-TAPI-NOT: _ZNK1Q5

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi marked 3 inline comments as done.
plotfi added a comment.

In D60974#1534662 , @rupprecht wrote:

> Can you upload this patch with context? Either use arc or upload w/ -U9


Thanks for the review.

> I seem to have a lot of comments, but they're all nits -- my major concern 
> being the `llvm_unreachable`s should be errors instead (i.e. should be 
> triggered in release builds).
> 
> Since this is clearly labeled as experimental, I think you should feel free 
> to commit if you can get another lgtm (@compnerd?)






Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3614-3616
+  Args.hasArg(options::OPT_iterface_stub_version_EQ)
+  ? Args.getLastArgValue(options::OPT_iterface_stub_version_EQ)
+  : "")

rupprecht wrote:
> `Args.getLastArgValue(options::OPT_iterface_stub_version_EQ)` should already 
> default to returning the empty string if unset (note the default param)
But what if it is set to something that's not either experimental-yaml-elf-v1 
or experimental-tapi-elf-v1? This was to truncate any values that aren't those 
two to "" so that the error check could just be if (StubFormat.empty()). Maybe 
something other than a string switch would have been more explicit. 



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1688
+  if (!ProgramActionPair.second)
+llvm_unreachable("Must specify a valid interface stub format.");
+  Opts.ProgramAction = ProgramActionPair.first;

rupprecht wrote:
> I think this is very much reachable if someone provides 
> `--interface-stub-version=x`
AH yes, you are right. This should probably be a diag. 
(clang/test/InterfaceStubs/bad-format.cpp tests for this "unreachable").



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:41
+  return true;
+if (Symbols.find(ND) != Symbols.end())
+  return true;

rupprecht wrote:
> llvm::is_contained(Symbols, ND)
llvm::is_contained does not appear to work with std::map. I will try to figure 
out why. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 204308.
plotfi marked 2 inline comments as done.
plotfi added a comment.

Improving support for visibility correctness with classes, methods and 
inheritance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// CHECK-T

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1690
+  std::make_pair(frontend::GenerateInterfaceTBEExpV1, false));
+  if (!ProgramActionPair.second)
+Diags.Report(diag::err_drv_invalid_value)

I think you could've used a `llvm::Optional`.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:21
+  StringRef InFile;
+  StringRef Format;
+  std::set ParsedTemplates;

An enumeration may be nicer.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:39
+  bool WriteNamedDecl(const NamedDecl *ND, MangledSymbols &Symbols, int RDO) {
+if (!(RDO & FromTU))
+  return true;

I tend to prefer `if (RDO & ~FromTU)`



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:56
+auto isVisible = [this](const NamedDecl *ND) -> bool {
+  if (ND->getVisibility() != DefaultVisibility) {
+if (ND->hasAttr())

Why not `if (ND->getVisibility() == DefaultVisibility) return true;`?



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:65
+};
+auto doBail = [this, isVisible](const NamedDecl *ND) -> bool {
+  if (!isVisible(ND))

I think that using something like `ignoreDecl` is better than `doBail`.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:68
+return true;
+  if (const VarDecl *VD = dyn_cast(ND))
+if ((VD->getStorageClass() == StorageClass::SC_Extern) ||

A newline before this would be nice.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:72
+ VD->getParentFunctionOrMethod() == nullptr))
+  return true;
+  if (const FunctionDecl *FD = dyn_cast(ND)) {

If it is a `VarDecl`, it cannot be a `FunctionDecl`.  I think that this can be 
simplified a bit as:

```
if (const auto *VD = dyn_cast(VD))
  return VD->getStorageClass() == StorageClass::SC_Extern ||
  VD->getStorageClass() == StorageClass::SC_Static ||
  VD->getParentFunctionOrMethod() == nullptr;
```



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:73
+  return true;
+  if (const FunctionDecl *FD = dyn_cast(ND)) {
+if (FD->isInlined() && !isa(FD) &&

Newline before this would be nice.  I think that using `auto` here for the type 
is fine as you spelt out the type in the `dyn_cast`.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:77
+  return true;
+if (const CXXMethodDecl *MD = dyn_cast(FD)) {
+  if (const auto *RC = dyn_cast(MD->getParent())) {

I think that flipping this around would be nicer.

```
if (const auto *MD = dyn_cast(FD)) {
  ...
}

return FD->isInlined() && !Instance.getLangOpts().GNUInline;
```



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:79
+  if (const auto *RC = dyn_cast(MD->getParent())) {
+if (const auto *CTD = dyn_cast(RC->getParent()))
+  return true;

`CTD` is not used, please use `isa` and avoid the unused variable warning.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:118
+  return MangledName;
+};
+

Can we compress `getMangledName` and `getMangledNames` into `getMangledNames` 
and always return the vector?  This avoids the duplication and simplifies the 
symbol recording later as well.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:150
+if (IsRDOLate)
+  llvm_unreachable("Generating Interface Stubs is not supported with "
+   "delayed template parsing.");

You can probably hoist this to L129, and avoid the variable definition and the 
check on L131.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 204978.
plotfi marked 17 inline comments as done.
plotfi added a comment.

Addressing @compnerd's feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// CHECK-TAPI-NOT: _ZNK1Q5func1Ev
+// CHECK-TAPI-NOT: _ZNK1Q5f

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added inline comments.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:21
+  StringRef InFile;
+  StringRef Format;
+  std::set ParsedTemplates;

compnerd wrote:
> An enumeration may be nicer.
For now the format goes into the first line of the yaml (in both formats) and 
ends up as "--- ! \n".
I'd like to keep it this way for now. 



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:39
+  bool WriteNamedDecl(const NamedDecl *ND, MangledSymbols &Symbols, int RDO) {
+if (!(RDO & FromTU))
+  return true;

compnerd wrote:
> I tend to prefer `if (RDO & ~FromTU)`
0 != (RDO & 0x1) vs (RDO & 0xE)

I'm not sure if these are logically equivalent



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:72
+ VD->getParentFunctionOrMethod() == nullptr))
+  return true;
+  if (const FunctionDecl *FD = dyn_cast(ND)) {

compnerd wrote:
> If it is a `VarDecl`, it cannot be a `FunctionDecl`.  I think that this can 
> be simplified a bit as:
> 
> ```
> if (const auto *VD = dyn_cast(VD))
>   return VD->getStorageClass() == StorageClass::SC_Extern ||
>   VD->getStorageClass() == StorageClass::SC_Static ||
>   VD->getParentFunctionOrMethod() == nullptr;
> ```
I don't understand. Here I remember wanting to bail if 
VD->getParentFunctionOrMethod() == nullptr and the vardecl was marked static. 



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:77
+  return true;
+if (const CXXMethodDecl *MD = dyn_cast(FD)) {
+  if (const auto *RC = dyn_cast(MD->getParent())) {

compnerd wrote:
> I think that flipping this around would be nicer.
> 
> ```
> if (const auto *MD = dyn_cast(FD)) {
>   ...
> }
> 
> return FD->isInlined() && !Instance.getLangOpts().GNUInline;
> ```
I think this still causes the isInlined + !GNUInlined code to trigger on 
CXXMethodDecls. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:91
+  std::vector MangledNames = CGNameGen.getAllManglings(ND);
+  if (isa(ND) || isa(ND)) {
+if (MangledNames.size() == 1)

I don't understand this clause.  The structurors will always have 2 manglings 
(both itanium and MSVC use multiple, I don't remember the SUN and GNU variants 
off the top of my head).  Adding the assert for the non-structor case is fine, 
but please wrap the entire thing in an `LLVM_EXPENSIVE_ASSERT`.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:116
+if (RDO & IsLate) {
+  llvm_unreachable("Generating Interface Stubs is not supported with "
+   "delayed template parsing.");

This really should emit a diagnostic.  The `CompilerInstance` should have a 
reference to the `DiagnosticsEngine`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 205191.
plotfi marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD2 %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD2 %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-CMD2-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-CMD-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-CMD-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-vers

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363626: [clang-ifs] Clang Interface Stubs, first version. 
(authored by zer0, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60974?vs=205191&id=205203#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974

Files:
  cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/Types.def
  cfe/trunk/include/clang/Frontend/FrontendActions.h
  cfe/trunk/include/clang/Frontend/FrontendOptions.h
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CMakeLists.txt
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  cfe/trunk/test/InterfaceStubs/bad-format.cpp
  cfe/trunk/test/InterfaceStubs/class-template-specialization.cpp
  cfe/trunk/test/InterfaceStubs/externstatic.c
  cfe/trunk/test/InterfaceStubs/function-template-specialization.cpp
  cfe/trunk/test/InterfaceStubs/hidden-class-inheritance.cpp
  cfe/trunk/test/InterfaceStubs/inline.c
  cfe/trunk/test/InterfaceStubs/inline.h
  cfe/trunk/test/InterfaceStubs/object.cpp
  cfe/trunk/test/InterfaceStubs/template-namespace-function.cpp
  cfe/trunk/test/InterfaceStubs/virtual.cpp
  cfe/trunk/test/InterfaceStubs/visibility.cpp
  cfe/trunk/test/InterfaceStubs/weak.cpp

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -623,6 +623,9 @@
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option]>, Group,
   HelpText<"Use the LLVM representation for assembler and object files">;
+def emit_iterface_stubs : Flag<["-"], "emit-interface-stubs">, Flags<[CC1Option]>, Group,
+  HelpText<"Generate Inteface Stub Files.">;
+def iterface_stub_version_EQ : JoinedOrSeparate<["-"], "interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
 def e : JoinedOrSeparate<["-"], "e">, Group;
 def fPIC : Flag<["-"], "fPIC">, Group;
Index: cfe/trunk/include/clang/Driver/Types.def
===
--- cfe/trunk/include/clang/Driver/Types.def
+++ cfe/trunk/include/clang/Driver/Types.def
@@ -88,6 +88,7 @@
 
 // Misc.
 TYPE("ast",  AST,  INVALID, "ast",   "u")
+TYPE("ifs",  IFS,  INVALID, "ifs",   "u")
 TYPE("pcm",  ModuleFile,   INVALID, "pcm",   "u")
 TYPE("plist",Plist,INVALID, "plist", "")
 TYPE("rewritten-objc",   RewrittenObjC,INVALID, "cpp",   "")
Index: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -220,6 +220,8 @@
 def err_module_header_file_invalid :
   Error<"unexpected module header file input '%0'">, DefaultFatal;
 
+def err_interface_stubs : Error<"clang-ifs (-emit-iterface-stubs): %0">;
+
 def err_test_module_file_extension_version : Error<
   "test module file extension '%0' has different version (%1.%2) than expected "
   "(%3.%4)">;
Index: cfe/trunk/include/clang/Frontend/FrontendOptions.h
===
--- cfe/trunk/include/clang/Frontend/FrontendOptions.h
+++ cfe/trunk/include/clang/Frontend/FrontendOptions.h
@@ -88,6 +88,10 @@
   /// Generate pre-compiled header.
   GeneratePCH,
 
+  /// Generate Interface Stub Files.
+  GenerateInterfaceYAMLExpV1,
+  GenerateInterfaceTBEExpV1,
+
   /// Only execute frontend initialization.
   InitOnly,
 
Index: cfe/trunk/include/clang/Frontend/FrontendActions.h
===
--- cfe/trunk/include/clang/Frontend/FrontendActions.h
+++ cfe/trunk/include/clang/Frontend/FrontendActions.h
@@ -119,6 +119,26 @@
   bool hasASTFileSupport() const override { return false; }
 };
 
+class GenerateInterfaceStubAction : public ASTFrontendAction {
+protected:
+  TranslationUnitKind getTranslationUnitKind() override { return TU_Module; }
+
+  bool hasASTFileSupport() const override { return false; }
+};
+
+// Support different interface stub formats this way:
+class GenerateInterfaceYAMLExpV1Action : public GenerateInterfaceStubAction {
+protected:
+  std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
+ StringRef InFile) override;
+};
+
+class GenerateInterfaceTBEExpV1Action : public GenerateInterfaceStubAction

[PATCH] D60974: Clang IFSO driver action.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: cfe/trunk/lib/Frontend/CMakeLists.txt:58
   clangDriver
+  clangIndex
   clangEdit

This is a layering issue. clangIndex depends on clangFrontend so clangFrontend 
should not depend on clangIndex.

The circular dependency is allowed for static libraries but it breaks 
`-DBUILD_SHARED_LIBS=on`:

```
CMake Error: The inter-target dependency graph contains the following strongly 
connected component (cycle):
  "clangFrontend" of type SHARED_LIBRARY
depends on "clangIndex" (weak)
  "clangIndex" of type SHARED_LIBRARY
depends on "clangFrontend" (weak)
At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies are 
allowed only among static libraries.
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi marked an inline comment as done.
plotfi added inline comments.



Comment at: cfe/trunk/lib/Frontend/CMakeLists.txt:58
   clangDriver
+  clangIndex
   clangEdit

MaskRay wrote:
> This is a layering issue. clangIndex depends on clangFrontend so 
> clangFrontend should not depend on clangIndex.
> 
> The circular dependency is allowed for static libraries but it breaks 
> `-DBUILD_SHARED_LIBS=on`:
> 
> ```
> CMake Error: The inter-target dependency graph contains the following 
> strongly connected component (cycle):
>   "clangFrontend" of type SHARED_LIBRARY
> depends on "clangIndex" (weak)
>   "clangIndex" of type SHARED_LIBRARY
> depends on "clangFrontend" (weak)
> At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies 
> are allowed only among static libraries.
> ```
Is there a bot affected by this? I will take a look.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:22
+  StringRef Format;
+  std::set ParsedTemplates;
+

Does `StringSet<>` work?



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:36
+  };
+  using MangledSymbols = std::map;
+

Are you relying on the ordered property of `std::map`?



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:101
+  return true;
+if (Symbols.find(ND) != Symbols.end())
+  return true;

`.count`



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:103
+  return true;
+// - Currently have not figured out how to produce the names for 
FieldDecls.
+// - Do not want to produce symbols for function paremeters.

This should be a TODO



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:124
+
+  Symbols.insert(std::make_pair(
+  ND,

If you use a llvm container, `emplace` or `try_emplace`


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi marked 3 inline comments as done.
plotfi added inline comments.



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:22
+  StringRef Format;
+  std::set ParsedTemplates;
+

MaskRay wrote:
> Does `StringSet<>` work?
It probably could work. I will likely follow up with an NFC patch for this. 



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:36
+  };
+  using MangledSymbols = std::map;
+

MaskRay wrote:
> Are you relying on the ordered property of `std::map`?
No I am not. 



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:101
+  return true;
+if (Symbols.find(ND) != Symbols.end())
+  return true;

MaskRay wrote:
> `.count`
Nice. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Alex Bradbury via Phabricator via cfe-commits]

asb added inline comments.



Comment at: cfe/trunk/lib/Frontend/CMakeLists.txt:58
   clangDriver
+  clangIndex
   clangEdit

plotfi wrote:
> MaskRay wrote:
> > This is a layering issue. clangIndex depends on clangFrontend so 
> > clangFrontend should not depend on clangIndex.
> > 
> > The circular dependency is allowed for static libraries but it breaks 
> > `-DBUILD_SHARED_LIBS=on`:
> > 
> > ```
> > CMake Error: The inter-target dependency graph contains the following 
> > strongly connected component (cycle):
> >   "clangFrontend" of type SHARED_LIBRARY
> > depends on "clangIndex" (weak)
> >   "clangIndex" of type SHARED_LIBRARY
> > depends on "clangFrontend" (weak)
> > At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies 
> > are allowed only among static libraries.
> > ```
> Is there a bot affected by this? I will take a look.
Sadly there is no -DBUILD_SHARED_LIBS=True bot. This layering issue will break 
builds for all downstream users using that option though.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi marked 3 inline comments as done.
plotfi added inline comments.



Comment at: cfe/trunk/lib/Frontend/CMakeLists.txt:58
   clangDriver
+  clangIndex
   clangEdit

asb wrote:
> plotfi wrote:
> > MaskRay wrote:
> > > This is a layering issue. clangIndex depends on clangFrontend so 
> > > clangFrontend should not depend on clangIndex.
> > > 
> > > The circular dependency is allowed for static libraries but it breaks 
> > > `-DBUILD_SHARED_LIBS=on`:
> > > 
> > > ```
> > > CMake Error: The inter-target dependency graph contains the following 
> > > strongly connected component (cycle):
> > >   "clangFrontend" of type SHARED_LIBRARY
> > > depends on "clangIndex" (weak)
> > >   "clangIndex" of type SHARED_LIBRARY
> > > depends on "clangFrontend" (weak)
> > > At least one of these targets is not a STATIC_LIBRARY.  Cyclic 
> > > dependencies are allowed only among static libraries.
> > > ```
> > Is there a bot affected by this? I will take a look.
> Sadly there is no -DBUILD_SHARED_LIBS=True bot. This layering issue will 
> break builds for all downstream users using that option though.
Just checking (to see if there is a bot to verify with). I have a fix (removing 
clangIndex) that I think will build with or without -DBUILD_SHARED_LIBS=ON. 



Comment at: cfe/trunk/lib/Frontend/CMakeLists.txt:58
   clangDriver
+  clangIndex
   clangEdit

plotfi wrote:
> asb wrote:
> > plotfi wrote:
> > > MaskRay wrote:
> > > > This is a layering issue. clangIndex depends on clangFrontend so 
> > > > clangFrontend should not depend on clangIndex.
> > > > 
> > > > The circular dependency is allowed for static libraries but it breaks 
> > > > `-DBUILD_SHARED_LIBS=on`:
> > > > 
> > > > ```
> > > > CMake Error: The inter-target dependency graph contains the following 
> > > > strongly connected component (cycle):
> > > >   "clangFrontend" of type SHARED_LIBRARY
> > > > depends on "clangIndex" (weak)
> > > >   "clangIndex" of type SHARED_LIBRARY
> > > > depends on "clangFrontend" (weak)
> > > > At least one of these targets is not a STATIC_LIBRARY.  Cyclic 
> > > > dependencies are allowed only among static libraries.
> > > > ```
> > > Is there a bot affected by this? I will take a look.
> > Sadly there is no -DBUILD_SHARED_LIBS=True bot. This layering issue will 
> > break builds for all downstream users using that option though.
> Just checking (to see if there is a bot to verify with). I have a fix 
> (removing clangIndex) that I think will build with or without 
> -DBUILD_SHARED_LIBS=ON. 
I've updated cfe/trunk/lib/Frontend/CMakeLists.txt in r363646


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

I had to back out r363646. While it worked for me building locally with and 
without shared lib, it appears to have caused bots to break. I will take 
another look at fixing it in about 10 hours.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:12
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Index/CodegenNameGenerator.h"
+#include "clang/Sema/TemplateInstCallback.h"

> rL363646

This file references symbols from clangIndex (`#include 
"clang/Index/CodegenNameGenerator.h"`), so you can't remove the `clangIndex` 
dependency from `CMakeFiles.txt`, otherwise `-DBUILD_SHARED_LIBS=off` builds 
would also fail. 

```
# Pass -Wl,-z,defs. This makes sure all symbols are defined. Otherwise a DSO
# build might work on ELF but fail on MachO/COFF.
```

The built shared library must have all of its dependencies specified on the 
linker command line.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

@plotfi Sorry I have to revert this patch. This can also cause problems in 
`-DBUILD_SHARED_LIBS=off` builds.  clangFrontend files cannot `#include 
"clang/Index/CodegenNameGenerator.h"`, I think you may have to move files 
around.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1547682 , @MaskRay wrote:

> @plotfi Sorry I have to revert this patch. This can also cause problems in 
> `-DBUILD_SHARED_LIBS=off` builds.  clangFrontend files cannot `#include 
> "clang/Index/CodegenNameGenerator.h"`, I think you may have to move files 
> around.


That’s fine. I will coordinate with you when I attempt to re-land.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 205686.
plotfi added a comment.

This should address the shared lib issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD2 %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD2 %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-CMD2-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-CMD-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-CMD-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+/

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

This was re-landed in r363948


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

The tests for this code are failing in my build.  Any suggestions welcome.

My setup:

  cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=/usr/bin/clang-7 
-DCMAKE_CXX_COMPILER=/usr/bin/clang++-7 -DCMAKE_C_FLAGS='-gmlt -Wall' 
-DCMAKE_CXX_FLAGS='-gmlt -Wall' -DLLVM_PARALLEL_LINK_JOBS=6 
-DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra' ../llvm-project/llvm

with head at:

  commit 3b7668ae4bb8fabf95bd78dc6a06ca75f6ec3958 (HEAD, origin/master, 
origin/HEAD)
  Author: Matt Arsenault 
  Date:   Mon Jul 1 13:34:26 2019 +
  
  AMDGPU/GlobalISel: Improve icmp selection coverage.
  
  Select s64 eq/ne scalar icmp.
  
  llvm-svn: 364765

The failures:

  ninja check-clang
  [3586/3587] Running the Clang regression tests
  llvm-lit: <...>/llvm-project/llvm/utils/lit/lit/llvm/config.py:340: note: 
using clang: <...>/build/bin/clang
  FAIL: Clang :: InterfaceStubs/virtual.cpp (6035 of 15113)
   TEST 'Clang :: InterfaceStubs/virtual.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 2';   <...>/build/bin/clang -target x86_64-unknown-linux-gnu 
-o - -emit-interface-stubs  -interface-stub-version=experimental-tapi-elf-v1 
<...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp |  
<...>/build/bin/FileCheck -check-prefix=CHECK-TAPI 
<...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp
  : 'RUN: at line 5';   <...>/build/bin/clang -target x86_64-unknown-linux-gnu 
-o - -emit-interface-stubs  -interface-stub-version=experimental-tapi-elf-v1 
<...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp |  
<...>/build/bin/FileCheck -check-prefix=CHECK-TAPI2 
<...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp
  : 'RUN: at line 8';   <...>/build/bin/clang -target x86_64-unknown-linux-gnu 
-o - -c <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp |  
llvm-readelf -s - 2>&1 | <...>/build/bin/FileCheck -check-prefix=CHECK-SYMBOLS 
<...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp:16:23: error: 
CHECK-SYMBOLS-DAG: expected string not found in input
  // CHECK-SYMBOLS-DAG: NOTYPE GLOBAL HIDDEN {{.*}} _ZNK1Q5func1Ev
^
  :1:1: note: scanning from here
  There are 28 section headers, starting at offset 0x718:
  ^
  :4:25: note: possible intended match here
   [Nr] Name Type Address Off Size ES Flg Lk Inf Al
  ^
  
  --
  
  
  FAIL: Clang :: InterfaceStubs/class-template-specialization.cpp (6041 of 
15113)
   TEST 'Clang :: 
InterfaceStubs/class-template-specialization.cpp' FAILED 
  Script:
  --
  : 'RUN: at line 2';   <...>/build/bin/clang -target x86_64-unknown-linux-gnu 
-o - -emit-interface-stubs  -interface-stub-version=experimental-tapi-elf-v1 
<...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp 
|  <...>/build/bin/FileCheck -check-prefix=CHECK-TAPI 
<...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
  : 'RUN: at line 6';   <...>/build/bin/clang -target x86_64-unknown-linux-gnu 
-o - -emit-interface-stubs  -interface-stub-version=experimental-tapi-elf-v1 
<...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp 
|  <...>/build/bin/FileCheck -check-prefix=CHECK-TAPI2 
<...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
  : 'RUN: at line 9';   <...>/build/bin/clang -target x86_64-unknown-linux-gnu 
-o - -c 
<...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp 
|  llvm-readelf -s - 2>&1 |  <...>/build/bin/FileCheck 
-check-prefix=CHECK-SYMBOLS 
<...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  
<...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp:26:23:
 error: CHECK-SYMBOLS-DAG: expected string not found in input
  // CHECK-SYMBOLS-DAG: FUNC GLOBAL DEFAULT {{[0-9]}} _Z1g
^
  :1:1: note: scanning from here
  There are 12 section headers, starting at offset 0x2a8:
  ^
  :7:41: note: possible intended match here
   [ 2] .text PROGBITS  40 17 00 AX 0 0 16
  ^
  
  --
  
  
  FAIL: Clang :: InterfaceStubs/hidden-class-inheritance.cpp (6044 of 15113)
   TEST 'Clang :: 
InterfaceStubs/hidden-class-inheritance.cpp' FAILED 
  Script:
  --
  : 'RUN: at line 2';   <...>/build/bin/clang -target x86_64-unknown-linux-gnu 
-o - -emit-interface-stubs  -interface-stub-version=experimental-tapi-elf-v1  
-DPARENT_CLASS_VISIBILITY="" -DCHILD_CLASS_VISIBILITY=""  
-DPARENT_METHOD_VISIBILITY="" -DCHILD_METHOD_VISIBILITY="" 
<...>/llvm-project/clang/test/InterfaceStubs/hidden-class-inheritance.cpp |  
<...>/build/bin/FileCheck -check-p

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1564747 , @ymandel wrote:

> The tests for this code are failing in my build.  Any suggestions welcome.
>
> My setup:
>
>   cmake -G Ninja -DCMAKE_BUILD_TYPE=Release 
> -DCMAKE_C_COMPILER=/usr/bin/clang-7 -DCMAKE_CXX_COMPILER=/usr/bin/clang++-7 
> -DCMAKE_C_FLAGS='-gmlt -Wall' -DCMAKE_CXX_FLAGS='-gmlt -Wall' 
> -DLLVM_PARALLEL_LINK_JOBS=6 -DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra' 
> ../llvm-project/llvm
>
>
> with head at:
>
>   commit 3b7668ae4bb8fabf95bd78dc6a06ca75f6ec3958 (HEAD, origin/master, 
> origin/HEAD)
>   Author: Matt Arsenault 
>   Date:   Mon Jul 1 13:34:26 2019 +
>  
>   AMDGPU/GlobalISel: Improve icmp selection coverage.
>  
>   Select s64 eq/ne scalar icmp.
>  
>   llvm-svn: 364765
>
>
> The failures:
>
>   ninja check-clang
>   [3586/3587] Running the Clang regression tests
>   llvm-lit: <...>/llvm-project/llvm/utils/lit/lit/llvm/config.py:340: note: 
> using clang: <...>/build/bin/clang
>   FAIL: Clang :: InterfaceStubs/virtual.cpp (6035 of 15113)
>    TEST 'Clang :: InterfaceStubs/virtual.cpp' FAILED 
> 
>   Script:
>   --
>   : 'RUN: at line 2';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -emit-interface-stubs  
> -interface-stub-version=experimental-tapi-elf-v1 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp |  
> <...>/build/bin/FileCheck -check-prefix=CHECK-TAPI 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp
>   : 'RUN: at line 5';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -emit-interface-stubs  
> -interface-stub-version=experimental-tapi-elf-v1 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp |  
> <...>/build/bin/FileCheck -check-prefix=CHECK-TAPI2 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp
>   : 'RUN: at line 8';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -c 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp |  llvm-readelf -s - 
> 2>&1 | <...>/build/bin/FileCheck -check-prefix=CHECK-SYMBOLS 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp
>   --
>   Exit Code: 1
>  
>   Command Output (stderr):
>   --
>   <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp:16:23: error: 
> CHECK-SYMBOLS-DAG: expected string not found in input
>   // CHECK-SYMBOLS-DAG: NOTYPE GLOBAL HIDDEN {{.*}} _ZNK1Q5func1Ev
> ^
>   :1:1: note: scanning from here
>   There are 28 section headers, starting at offset 0x718:
>   ^
>   :4:25: note: possible intended match here
>[Nr] Name Type Address Off Size ES Flg Lk Inf Al
>   ^
>  
>   --
>  
>   
>   FAIL: Clang :: InterfaceStubs/class-template-specialization.cpp (6041 of 
> 15113)
>    TEST 'Clang :: 
> InterfaceStubs/class-template-specialization.cpp' FAILED 
>   Script:
>   --
>   : 'RUN: at line 2';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -emit-interface-stubs  
> -interface-stub-version=experimental-tapi-elf-v1 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>  |  <...>/build/bin/FileCheck -check-prefix=CHECK-TAPI 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>   : 'RUN: at line 6';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -emit-interface-stubs  
> -interface-stub-version=experimental-tapi-elf-v1 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>  |  <...>/build/bin/FileCheck -check-prefix=CHECK-TAPI2 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>   : 'RUN: at line 9';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -c 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>  |  llvm-readelf -s - 2>&1 |  <...>/build/bin/FileCheck 
> -check-prefix=CHECK-SYMBOLS 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>   --
>   Exit Code: 1
>  
>   Command Output (stderr):
>   --
>   
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp:26:23:
>  error: CHECK-SYMBOLS-DAG: expected string not found in input
>   // CHECK-SYMBOLS-DAG: FUNC GLOBAL DEFAULT {{[0-9]}} _Z1g
> ^
>   :1:1: note: scanning from here
>   There are 12 section headers, starting at offset 0x2a8:
>   ^
>   :7:41: note: possible intended match here
>[ 2] .text PROGBITS  40 17 00 AX 0 0 16
>   ^
>  
>   --
>  
>   
>   FAIL: Clang :: InterfaceStubs/hidden-class-inheritance.cpp (6044 of 15113)
>    TEST 'Clang :: 
> InterfaceStubs/hidden-class-inheritance.cpp' FAILED 
>   Script:
>   --
>   : 'RUN: at line 2';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1564747 , @ymandel wrote:

> The tests for this code are failing in my build.  Any suggestions welcome.
>
> My setup:
>
>   cmake -G Ninja -DCMAKE_BUILD_TYPE=Release 
> -DCMAKE_C_COMPILER=/usr/bin/clang-7 -DCMAKE_CXX_COMPILER=/usr/bin/clang++-7 
> -DCMAKE_C_FLAGS='-gmlt -Wall' -DCMAKE_CXX_FLAGS='-gmlt -Wall' 
> -DLLVM_PARALLEL_LINK_JOBS=6 -DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra' 
> ../llvm-project/llvm
>
>
> with head at:
>
>   commit 3b7668ae4bb8fabf95bd78dc6a06ca75f6ec3958 (HEAD, origin/master, 
> origin/HEAD)
>   Author: Matt Arsenault 
>   Date:   Mon Jul 1 13:34:26 2019 +
>  
>   AMDGPU/GlobalISel: Improve icmp selection coverage.
>  
>   Select s64 eq/ne scalar icmp.
>  
>   llvm-svn: 364765
>
>
> The failures:
>
>   ninja check-clang
>   [3586/3587] Running the Clang regression tests
>   llvm-lit: <...>/llvm-project/llvm/utils/lit/lit/llvm/config.py:340: note: 
> using clang: <...>/build/bin/clang
>   FAIL: Clang :: InterfaceStubs/virtual.cpp (6035 of 15113)
>    TEST 'Clang :: InterfaceStubs/virtual.cpp' FAILED 
> 
>   Script:
>   --
>   : 'RUN: at line 2';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -emit-interface-stubs  
> -interface-stub-version=experimental-tapi-elf-v1 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp |  
> <...>/build/bin/FileCheck -check-prefix=CHECK-TAPI 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp
>   : 'RUN: at line 5';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -emit-interface-stubs  
> -interface-stub-version=experimental-tapi-elf-v1 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp |  
> <...>/build/bin/FileCheck -check-prefix=CHECK-TAPI2 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp
>   : 'RUN: at line 8';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -c 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp |  llvm-readelf -s - 
> 2>&1 | <...>/build/bin/FileCheck -check-prefix=CHECK-SYMBOLS 
> <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp
>   --
>   Exit Code: 1
>  
>   Command Output (stderr):
>   --
>   <...>/llvm-project/clang/test/InterfaceStubs/virtual.cpp:16:23: error: 
> CHECK-SYMBOLS-DAG: expected string not found in input
>   // CHECK-SYMBOLS-DAG: NOTYPE GLOBAL HIDDEN {{.*}} _ZNK1Q5func1Ev
> ^
>   :1:1: note: scanning from here
>   There are 28 section headers, starting at offset 0x718:
>   ^
>   :4:25: note: possible intended match here
>[Nr] Name Type Address Off Size ES Flg Lk Inf Al
>   ^
>  
>   --
>  
>   
>   FAIL: Clang :: InterfaceStubs/class-template-specialization.cpp (6041 of 
> 15113)
>    TEST 'Clang :: 
> InterfaceStubs/class-template-specialization.cpp' FAILED 
>   Script:
>   --
>   : 'RUN: at line 2';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -emit-interface-stubs  
> -interface-stub-version=experimental-tapi-elf-v1 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>  |  <...>/build/bin/FileCheck -check-prefix=CHECK-TAPI 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>   : 'RUN: at line 6';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -emit-interface-stubs  
> -interface-stub-version=experimental-tapi-elf-v1 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>  |  <...>/build/bin/FileCheck -check-prefix=CHECK-TAPI2 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>   : 'RUN: at line 9';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-gnu -o - -c 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>  |  llvm-readelf -s - 2>&1 |  <...>/build/bin/FileCheck 
> -check-prefix=CHECK-SYMBOLS 
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp
>   --
>   Exit Code: 1
>  
>   Command Output (stderr):
>   --
>   
> <...>/llvm-project/clang/test/InterfaceStubs/class-template-specialization.cpp:26:23:
>  error: CHECK-SYMBOLS-DAG: expected string not found in input
>   // CHECK-SYMBOLS-DAG: FUNC GLOBAL DEFAULT {{[0-9]}} _Z1g
> ^
>   :1:1: note: scanning from here
>   There are 12 section headers, starting at offset 0x2a8:
>   ^
>   :7:41: note: possible intended match here
>[ 2] .text PROGBITS  40 17 00 AX 0 0 16
>   ^
>  
>   --
>  
>   
>   FAIL: Clang :: InterfaceStubs/hidden-class-inheritance.cpp (6044 of 15113)
>    TEST 'Clang :: 
> InterfaceStubs/hidden-class-inheritance.cpp' FAILED 
>   Script:
>   --
>   : 'RUN: at line 2';   <...>/build/bin/clang -target 
> x86_64-unknown-linux-

[PATCH] D60974: Clang IFSO driver action.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D60974#1565054 , @plotfi wrote:

> So I think I know what may be going on on your end. The llvm-readelf in your 
> path I believe might be the wrong lllvm-readelf (llvm-readelf-7). Are you 
> sure you built llvm-readelf from git?


Right -- no, I'm not. I explicitly added the llvm-7 bin to my path because 
without it, those tests fail with:

  build/tools/clang/test/InterfaceStubs/Output/visibility.cpp.script: line 7: 
llvm-readelf: command not found

Sorry I didn't mention this up front.  I've never needed llvm-readelf before -- 
what is the standard way to build/install from git?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

`ninja install` did the trick.  Thanks for your help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D60974#1565054 , @plotfi wrote:

> So I think I know what may be going on on your end. The llvm-readelf in your 
> path I believe might be the wrong lllvm-readelf (llvm-readelf-7). Are you 
> sure you built llvm-readelf from git?


Please don't do this. Your commit is wrong, and the right action from you is to 
revert it or fix it. I've fixed it for you here: rL364855 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D60974#1565517 , @jfb wrote:

> In D60974#1565054 , @plotfi wrote:
>
> > So I think I know what may be going on on your end. The llvm-readelf in 
> > your path I believe might be the wrong lllvm-readelf (llvm-readelf-7). Are 
> > you sure you built llvm-readelf from git?
>
>
> Please don't do this. Your commit is wrong, and the right action from you is 
> to revert it or fix it. I've fixed it for you here: rL364855 
> 


I'm also really not sure this is any good: why does clang look at ELF? In 
general I'd expect you to test something *way* earlier than ELF. Are you sure 
you're testing the right thing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1565527 , @jfb wrote:

> In D60974#1565517 , @jfb wrote:
>
> > In D60974#1565054 , @plotfi wrote:
> >
> > > So I think I know what may be going on on your end. The llvm-readelf in 
> > > your path I believe might be the wrong lllvm-readelf (llvm-readelf-7). 
> > > Are you sure you built llvm-readelf from git?
> >
> >
> > Please don't do this. Your commit is wrong, and the right action from you 
> > is to revert it or fix it. I've fixed it for you here: rL364855 
> > 
>
>
> I'm also really not sure this is any good: why does clang look at ELF? In 
> general I'd expect you to test something *way* earlier than ELF. Are you sure 
> you're testing the right thing?


Ah, I must have mistakenly thought that other tools were already using 
llvm-readelf in cfe. Sorry about this.

This is in fact intended. The interface stubs feature needs to be verified 
against the actual symbols that are emitted into the elf binary. In the cases 
where I use llvm-readelf, I am using it to determine if the symbol is visible 
(to nm for example) but marked hidden in the .o file (but will end up being 
hidden in when linked).

Currently clang -emit-interface-stubs is not using any elf libraries, but it 
needs to verify against llvm-nm and/or llvm-readelf in these lit tests to 
determine if the visibility of the symbols generated in the text stubs is 
correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D60974#1565541 , @plotfi wrote:

> In D60974#1565527 , @jfb wrote:
>
> > In D60974#1565517 , @jfb wrote:
> >
> > > In D60974#1565054 , @plotfi 
> > > wrote:
> > >
> > > > So I think I know what may be going on on your end. The llvm-readelf in 
> > > > your path I believe might be the wrong lllvm-readelf (llvm-readelf-7). 
> > > > Are you sure you built llvm-readelf from git?
> > >
> > >
> > > Please don't do this. Your commit is wrong, and the right action from you 
> > > is to revert it or fix it. I've fixed it for you here: rL364855 
> > > 
> >
> >
> > I'm also really not sure this is any good: why does clang look at ELF? In 
> > general I'd expect you to test something *way* earlier than ELF. Are you 
> > sure you're testing the right thing?
>
>
> Ah, I must have mistakenly thought that other tools were already using 
> llvm-readelf in cfe. Sorry about this.
>
> This is in fact intended. The interface stubs feature needs to be verified 
> against the actual symbols that are emitted into the elf binary. In the cases 
> where I use llvm-readelf, I am using it to determine if the symbol is visible 
> (to nm for example) but marked hidden in the .o file (but will end up being 
> hidden in when linked).
>
> Currently clang -emit-interface-stubs is not using any elf libraries, but it 
> needs to verify against llvm-nm and/or llvm-readelf in these lit tests to 
> determine if the visibility of the symbols generated in the text stubs is 
> correct.


`llvm-nm` is already in the list of requirement for clang, so that would be 
fine to use. I'm not sure `llvm-readelf` is necessarily built for all targets, 
so my "fix" might still be broken. It's also another dependency, and a weird 
one at that. Can you test the property you're looking for in IR directly? And 
then test, in LLVM, that the same IR generates the ELF binary you want?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1565544 , @jfb wrote:

> In D60974#1565541 , @plotfi wrote:
>
> > In D60974#1565527 , @jfb wrote:
> >
> > > In D60974#1565517 , @jfb wrote:
> > >
> > > > In D60974#1565054 , @plotfi 
> > > > wrote:
> > > >
> > > > > So I think I know what may be going on on your end. The llvm-readelf 
> > > > > in your path I believe might be the wrong lllvm-readelf 
> > > > > (llvm-readelf-7). Are you sure you built llvm-readelf from git?
> > > >
> > > >
> > > > Please don't do this. Your commit is wrong, and the right action from 
> > > > you is to revert it or fix it. I've fixed it for you here: rL364855 
> > > > 
> > >
> > >
> > > I'm also really not sure this is any good: why does clang look at ELF? In 
> > > general I'd expect you to test something *way* earlier than ELF. Are you 
> > > sure you're testing the right thing?
> >
> >
> > Ah, I must have mistakenly thought that other tools were already using 
> > llvm-readelf in cfe. Sorry about this.
> >
> > This is in fact intended. The interface stubs feature needs to be verified 
> > against the actual symbols that are emitted into the elf binary. In the 
> > cases where I use llvm-readelf, I am using it to determine if the symbol is 
> > visible (to nm for example) but marked hidden in the .o file (but will end 
> > up being hidden in when linked).
> >
> > Currently clang -emit-interface-stubs is not using any elf libraries, but 
> > it needs to verify against llvm-nm and/or llvm-readelf in these lit tests 
> > to determine if the visibility of the symbols generated in the text stubs 
> > is correct.
>
>
> `llvm-nm` is already in the list of requirement for clang, so that would be 
> fine to use. I'm not sure `llvm-readelf` is necessarily built for all 
> targets, so my "fix" might still be broken. It's also another dependency, and 
> a weird one at that. Can you test the property you're looking for in IR 
> directly? And then test, in LLVM, that the same IR generates the ELF binary 
> you want?


I am currently working on the next part of clang interface stubs that will take 
the interface stubs per compilation unit and merge them into one text stub 
(which will be used by something like llvm-elfabi to generate a stubbed out ELF 
.so file). I was using llvm-nm, but there are cases where the symbol is present 
in the .o file but it is marked as HIDDEN and llvm-readelf was what I was using 
to determine if the symbol was in fact marked as hidden. In these cases, the 
linker will not emit the symbol in the final .so file but it still needs the 
symbol as part of linking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

@ymandel @jfb Thanks for reaching out on this. Apologies for missing this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1565544 , @jfb wrote:

> In D60974#1565541 , @plotfi wrote:
>
> > In D60974#1565527 , @jfb wrote:
> >
> > > In D60974#1565517 , @jfb wrote:
> > >
> > > > In D60974#1565054 , @plotfi 
> > > > wrote:
> > > >
> > > > > So I think I know what may be going on on your end. The llvm-readelf 
> > > > > in your path I believe might be the wrong lllvm-readelf 
> > > > > (llvm-readelf-7). Are you sure you built llvm-readelf from git?
> > > >
> > > >
> > > > Please don't do this. Your commit is wrong, and the right action from 
> > > > you is to revert it or fix it. I've fixed it for you here: rL364855 
> > > > 
> > >
> > >
> > > I'm also really not sure this is any good: why does clang look at ELF? In 
> > > general I'd expect you to test something *way* earlier than ELF. Are you 
> > > sure you're testing the right thing?
> >
> >
> > Ah, I must have mistakenly thought that other tools were already using 
> > llvm-readelf in cfe. Sorry about this.
> >
> > This is in fact intended. The interface stubs feature needs to be verified 
> > against the actual symbols that are emitted into the elf binary. In the 
> > cases where I use llvm-readelf, I am using it to determine if the symbol is 
> > visible (to nm for example) but marked hidden in the .o file (but will end 
> > up being hidden in when linked).
> >
> > Currently clang -emit-interface-stubs is not using any elf libraries, but 
> > it needs to verify against llvm-nm and/or llvm-readelf in these lit tests 
> > to determine if the visibility of the symbols generated in the text stubs 
> > is correct.
>
>
> `llvm-nm` is already in the list of requirement for clang, so that would be 
> fine to use. I'm not sure `llvm-readelf` is necessarily built for all 
> targets, so my "fix" might still be broken. It's also another dependency, and 
> a weird one at that. Can you test the property you're looking for in IR 
> directly? And then test, in LLVM, that the same IR generates the ELF binary 
> you want?


FWIW, It's possible that when I finish the merge job part of clang interface 
stubs I can remove this llvm-readelf test dependency because I can just check 
against llvm-nm that the merged text stub is correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.



> I am currently working on the next part of clang interface stubs that will 
> take the interface stubs per compilation unit and merge them into one text 
> stub (which will be used by something like llvm-elfabi to generate a stubbed 
> out ELF .so file). I was using llvm-nm, but there are cases where the symbol 
> is present in the .o file but it is marked as HIDDEN and llvm-readelf was 
> what I was using to determine if the symbol was in fact marked as hidden. In 
> these cases, the linker will not emit the symbol in the final .so file but it 
> still needs the symbol as part of linking.

This is similar to using "llvm-bcanalyzer" to check for serialization stuff 
from clang. We can't guarantee that to be available and adding a dep here would 
be weird. One way to workaround that would be to have a %llvm-readelf thing in 
lit and not run tests if not available (which also makes it questionable if 
it's not always running, but better than nothing?). Another option is to add a 
small clang tool that dumps what you need to check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Looking at the code quickly, I'm not sure that this should be in clang itself. 
It sounds like a better fit for a clang-based tool, and not clang. Why does it 
need to be part of clang?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1565577 , @jfb wrote:

> Looking at the code quickly, I'm not sure that this should be in clang 
> itself. It sounds like a better fit for a clang-based tool, and not clang. 
> Why does it need to be part of clang?




In D60974#1565577 , @jfb wrote:

> Looking at the code quickly, I'm not sure that this should be in clang 
> itself. It sounds like a better fit for a clang-based tool, and not clang. 
> Why does it need to be part of clang?


@jfb Having interface stubs generated by clang itself makes sense because we 
want to use the same clang invocation (with the addition of 
-emit-interface-stubs) to be what generates the stubs so that everything is 
fully consistent with what -fvisibility/__attribute__ visibility would have 
exposed in a normal non-stub build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1565570 , @bruno wrote:

> > I am currently working on the next part of clang interface stubs that will 
> > take the interface stubs per compilation unit and merge them into one text 
> > stub (which will be used by something like llvm-elfabi to generate a 
> > stubbed out ELF .so file). I was using llvm-nm, but there are cases where 
> > the symbol is present in the .o file but it is marked as HIDDEN and 
> > llvm-readelf was what I was using to determine if the symbol was in fact 
> > marked as hidden. In these cases, the linker will not emit the symbol in 
> > the final .so file but it still needs the symbol as part of linking.
>
> This is similar to using "llvm-bcanalyzer" to check for serialization stuff 
> from clang. We can't guarantee that to be available and adding a dep here 
> would be weird. One way to workaround that would be to have a %llvm-readelf 
> thing in lit and not run tests if not available (which also makes it 
> questionable if it's not always running, but better than nothing?). Another 
> option is to add a small clang tool that dumps what you need to check.


I will take a look at this. Thanks @bruno


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1565577 , @jfb wrote:

> Looking at the code quickly, I'm not sure that this should be in clang 
> itself. It sounds like a better fit for a clang-based tool, and not clang. 
> Why does it need to be part of clang?


@jfb I think this could actually be refactored into a clang based tool come to 
think of it, but I am also looking at how to add some features to the driver so 
that it can support job pipelines that are different from the standard PP -> 
CC1 -> BE -> ASM -> LINK pipeline. I can work on a change once I sort out this 
pipeline work to move the interface generation into a clang tool so that 
instead of invoking clang -cc1 to generate the interfaces the clang driver 
invokes the new clang interface generation tool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[JF Bastien via Phabricator via cfe-commits]

jfb added subscribers: arphaman, ributzka, Bigcheese.
jfb added a comment.

In D60974#1565621 , @plotfi wrote:

> In D60974#1565577 , @jfb wrote:
>
> > Looking at the code quickly, I'm not sure that this should be in clang 
> > itself. It sounds like a better fit for a clang-based tool, and not clang. 
> > Why does it need to be part of clang?
>
>
> @jfb I think this could actually be refactored into a clang based tool come 
> to think of it, but I am also looking at how to add some features to the 
> driver so that it can support job pipelines that are different from the 
> standard PP -> CC1 -> BE -> ASM -> LINK pipeline. I can work on a change once 
> I sort out this pipeline work to move the interface generation into a clang 
> tool so that instead of invoking clang -cc1 to generate the interfaces the 
> clang driver invokes the new clang interface generation tool.


That sounds pretty good, thanks! Maybe someone like @ributzka / @arphaman / 
@Bigcheese have opinions on how to best do this.
Once you do this, can you also revert rL364855 
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1565632 , @jfb wrote:

> In D60974#1565621 , @plotfi wrote:
>
> > In D60974#1565577 , @jfb wrote:
> >
> > > Looking at the code quickly, I'm not sure that this should be in clang 
> > > itself. It sounds like a better fit for a clang-based tool, and not 
> > > clang. Why does it need to be part of clang?
> >
> >
> > @jfb I think this could actually be refactored into a clang based tool come 
> > to think of it, but I am also looking at how to add some features to the 
> > driver so that it can support job pipelines that are different from the 
> > standard PP -> CC1 -> BE -> ASM -> LINK pipeline. I can work on a change 
> > once I sort out this pipeline work to move the interface generation into a 
> > clang tool so that instead of invoking clang -cc1 to generate the 
> > interfaces the clang driver invokes the new clang interface generation tool.
>
>
> That sounds pretty good, thanks! Maybe someone like @ributzka / @arphaman / 
> @Bigcheese have opinions on how to best do this.
>  Once you do this, can you also revert rL364855 
> ?


That is definitely possible, because then only the clang-tool that emits the 
interfaces needs to verify against llvm-readelf. The the driver invocation path 
can just test against the llvm-nm output of the final .so file. More 
collaboration and feedback on this project is very welcome.

Currently @compnerd and I have been discussing allowing for a more flexible 
compilation pipeline generation, where possibly instead of the standard PP -> 
CC -> BE -> ASM -> LNK we might have something like PP -> clang-ifsgen -> 
llvm-ifsmerge -> llvm-elfabi.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi created this revision.
Herald added subscribers: cfe-commits, eraman, mgorny.
Herald added a project: clang.

This enables -emit-ifso to generate an interface library for each .o file. 
Currently it just writes a text file with the mangled names in it.


Repository:
  rC Clang

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/IFSO/foo-inline.h
  clang/test/IFSO/foo.cpp

Index: clang/test/IFSO/foo.cpp
===
--- /dev/null
+++ clang/test/IFSO/foo.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang -emit-ifso -fvisibility=hidden %s -o - | FileCheck --check-prefix=CHECK-HIDDEN %s
+// RUN: %clang -emit-ifso %s -o - | FileCheck %s
+
+// CHECK-HIDDEN-NOT: __Z4fbarff
+// CHECK: __Z4fbarff
+
+
+
+
+// CHECK-HIDDEN-NOT: __Z3fooii
+// CHECK-NOT:__Z3fooii
+
+
+
+#include "foo-inline.h"
+
+__attribute__ ((visibility ("hidden"))) int foo(int a, int b) { return a + b; }
+__attribute__ ((visibility ("default"))) int foo_default_visi(int a, int b) { return a + b; }
+
+
+__attribute__ ((visibility ("default"))) int fvih_1(int a, int b) { return a + fvih(); }
+
+int dataA = 34;
+
+namespace baz {
+  template 
+  T add(T a, T b) {
+return a + b;
+  }
+}
+
+namespace n {
+  template 
+  struct __attribute__((__visibility__("default"))) S {
+S() = default;
+~S() = default;
+int __attribute__((__visibility__(("default" func() const { return 32; }
+int __attribute__((__visibility__(("hidden" operator()() const { return 53; }
+  };
+}
+
+template  T neverUsed(T t) { return t + 2; }
+
+template<> int neverUsed(int t);
+
+void g() { n::S()(); }
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+}
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/IFSO/foo-inline.h
===
--- /dev/null
+++ clang/test/IFSO/foo-inline.h
@@ -0,0 +1,6 @@
+
+inline int fvih() {
+static int fortytwo = 42;
+  return fortytwo;
+}
+
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -64,6 +64,7 @@
   case GenerateHeaderModule:
 return llvm::make_unique();
   case GeneratePCH:return llvm::make_unique();
+  case GenerateIFSO:   return llvm::make_unique();
   case InitOnly:   return llvm::make_unique();
   case ParseSyntaxOnly:return llvm::make_unique();
   case ModuleFileInfo: return llvm::make_unique();
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -25,6 +25,8 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Index/CodegenNameGenerator.h"
 #include 
 #include 
 
@@ -158,6 +160,136 @@
   return true;
 }
 
+class IFSOFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;
+  StringRef InFile = "";
+  std::set ParsedTemplates;
+
+  enum RootDeclOrigin { TopLevel = 0, FromTU = 1, IsLate = 2 };
+
+  template 
+  bool WriteNamedDecl(const NamedDecl *ND, raw_pwrite_stream &OS, int RDO) {
+if (!isa(ND))
+  return false;
+if (ND->getVisibility() != DefaultVisibility)
+  return true;
+// If this is a FunctionDecl that is dependent on a template parameter, then
+// don't get the symbol because we can only export specializations.
+bool IsRDOLate = (RDO & IsLate);
+if (const auto *FD = dyn_cast(ND))
+  if (FD->isDependentContext() && !IsRDOLate)
+return true;
+index::CodegenNameGenerator CGNameGen(ND->getASTContext());
+std::string MangledName = CGNameGen.getName(ND);
+OS << (IsRDOLate ? "late-parsed-decl: " : "")
+   << (IsRDOLate ? ND->getNameAsString() : MangledName) << "\n";
+// For now, lets just dump the -fdelayed-template-parsing decls until we
+// decide how to handle them.
+if (IsRDOLate)
+  ND->dump();
+return true;
+  }
+
+  template 
+  bool HandleSomeDecl(const NamedDecl *ND, raw_pwrite_stream &OS,
+  int RDO) {
+if (!isa(ND))
+  return false;
+for (auto *I : cast(ND)->decls())
+  HandleNamedDecl(dyn_cast(I), OS, RDO);
+return true;
+  }
+
+  template 
+  bool HandleSomeDeclSpec(const NamedDecl *ND,

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi marked an inline comment as done.
plotfi added inline comments.



Comment at: clang/lib/Frontend/FrontendActions.cpp:185
+OS << (IsRDOLate ? "late-parsed-decl: " : "")
+   << (IsRDOLate ? ND->getNameAsString() : MangledName) << "\n";
+// For now, lets just dump the -fdelayed-template-parsing decls until we

Still feeling out what the proper format should be. Might just go with yaml for 
now. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

Can you elaborate on the use case for this? Like can you explain end to end how 
this would be used?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1474591 , @jakehehrlich 
wrote:

> Can you elaborate on the use case for this? Like can you explain end to end 
> how this would be used?


There are a few that I have in mind.

1. -emit-ifso could be added to a build to produce .so files as part of a 
device SDK (where we don't want to ship the runnable bits in the SDK, we ship 
those on the device updates).
2. -emit-ifso could be added to whatever the existing set of build flags are to 
be run as a pre-build step to produce tapi-like .so files that break up build 
dependencies.
3. -emit-ifso -fvisibility=hidden could added to the invocation to disallow 
usage of non-public apis.

The way we are thinking it would work is similar to the dwos and .o files where 
you get one .ifso for each TU, then have a link-action that merges the mangled 
names and produces the .so file.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

In D60974#1474751 , @plotfi wrote:

> There are a few that I have in mind.
>
> 1. -emit-ifso could be added to a build to produce .so files as part of a 
> device SDK (where we don't want to ship the runnable bits in the SDK, we ship 
> those on the device updates).


This would require creating a linker for these files. Is this what you intend 
to do?

> 2. -emit-ifso could be added to whatever the existing set of build flags are 
> to be run as a pre-build step to produce tapi-like .so files that break up 
> build dependencies.

How would this work? To be clear, this would happen at the same time that .o 
files are produced correct? The build graph would basically look the same but 
your new linking step would replace the original link step and there would be a 
pendent node for the original link step. This would not break up the build 
graph much at all save for the fact that we might imagine that this new link 
step would be much faster.

> 3. -emit-ifso -fvisibility=hidden could added to the invocation to disallow 
> usage of non-public apis.

Can you be more specific? This is already how the actual modules do this today 
so what is the advantage here.

> The way we are thinking it would work is similar to the dwos and .o files 
> where you get one .ifso for each TU, then have a link-action that merges the 
> mangled names and produces the .so file.

That requires a linker but one that only has to link dwarf which is designed to 
be linked using the dumbest possible semantics so that it can be format 
agnostic and not create undo complexity in the linker. The linker needed for 
this sort of thing would have to understand almost everything that a real full 
linker has to just to determine if a symbol is public or not. I think this 
might even require processing relocations. Beyond being public symbols have 
extra information that matters at both static and dynamic link time. This is 
mostly just point out how complicated this task is and not a complaint about 
the current output format. That could be improved but we have to be aware of 
the complexity of introducing yet a third type of linker and the issues with 
maintaining parity with the behavior of existing linker as well. In particular 
I think introducing a linker is an even bigger llvm-dev discussion that we need 
to have before we should proceed with this method.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

@jakehehrlich I think you are misunderstanding this.  The intent here is to 
provide a means to emit just the interfaces from a library into a stub that can 
be used for building.  However, rather than have the interfaces be defined 
elsewhere, having clang run through the sources and compute the public 
interfaces as written means that no matter how you write your interfaces, the 
same set of interfaces will always be emitted into the interface library.  The 
"linker" literally is a merge of all the public interfaces in each translation 
unit.  It is important to understand that there are interfaces which may be 
declared in sources (yes, that is terrible, but not all software is well 
designed or well written unfortunately).  This means that you need to do a 
syntactic pass of the sources (along with the flags that you are building with) 
to ensure that all the public interfaces are made visible.  You can do this by 
a separate step of consuming the compilation database and running through each 
of the target's TUs, this is just a slightly different approach to that.  Once 
you have enumerated the public interfaces, you just emit a stub containing that 
set of interfaces (preserving whether it is a text or data symbol).

I'm not sure how modules play into this at all.  Yes, C++ modules could let you 
control some of the interfaces, but that is orthogonal to what this is meant to 
do.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

I'm not misunderstanding, please seem my post on llvm-dev where I layout the 
different issues involved. You're underestimating the complexities involved 
here I think. Trivializing what the linker does to "the linker just merges 
these" is a gross oversimplification that doesn't account for symbol consuming 
vs exporting, size, alignment, visibility, linkage, symbol type, symbol 
versioning (which is impossible to handle here without consuming version 
scripts as was pointed out by someone else), which symbols are untimely 
exported/consumed or even intermediate operations on .o files via tools like 
objcopy (which is uncommon and shouldn't be a hard blocker here but something 
to think about). Also all linkers behave just a bit differently with respect to 
these issues so 1) the reality is that the thing that merges these will behave 
just a bit differently than any existing linker but 2) even if it trys to match 
one linker it will fail to match the other so it will only be compaitable in 
all cases with atmost one linker (but probably none when edge cases are 
considered. I also already pointed out the issue with having to search all 
source files on the tree. I agree that if you were to use the linker to 
accomplish this that would would have to look at all source files. Further more 
many flags passed to the linker untimely affect these things making matters 
more complicated. The "linker" you describe for these cases is far from "cat".

For reference when I say "module" in this context I mean the output of the 
linker. "DSO" is synonymous. A DSO/module can be an executable or shared object 
but we can consider both here. I was not referring to C++ modules.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

I'm well versed in the complexities of a linker - I've worked extensively on 
the GNU linkers as well as with lld.  The visibility of the symbols is actually 
computed by the compiler - the STV_* flags associated with the symtab entry 
give you that information which is actually tracked through the frontend to the 
backend.  Yes, each linker behaves differently - including lld which completely 
changes the semantics of Unix/ELF linking guarantees.  In fact every single 
attribute that you mentioned is something which is emitted from the compiler - 
which we have access to here given that we are in clang itself.  I think that 
this can be made to work properly, though will probably require some iteration 
to get all the corner cases, and that you may be slightly dismissive of this 
approach.

Ah, okay, I didn't pick up that you were using module as the actual module from 
the context since normally the module doesn't control what it exposes itself.

Note that I am not advocating that this change go in as is - I think that this 
is far from something which would be useful, and until it can produce something 
meaningful, this shouldn't really be merged (i.e. generate something which 
obj2yaml or some other tool can consume to generate the barest ELF stub).


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 196291.
plotfi added a comment.
Herald added a subscriber: mgrang.

I've added an output format. The YAML coming out of -emit-ifso can now run 
through yaml2obj to produce a striped .o file. Those .o files can be handled by 
an existing linker.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/IFSO/foo-inline.h
  clang/test/IFSO/foo.cpp

Index: clang/test/IFSO/foo.cpp
===
--- /dev/null
+++ clang/test/IFSO/foo.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang -emit-ifso -fvisibility=hidden %s -o - | FileCheck --check-prefix=CHECK-HIDDEN %s
+// RUN: %clang -emit-ifso %s -o - | FileCheck %s
+
+// CHECK-HIDDEN-NOT: __Z4fbarff
+// CHECK: __Z4fbarff
+
+
+
+
+// CHECK-HIDDEN-NOT: __Z3fooii
+// CHECK-NOT:__Z3fooii
+
+
+
+#include "foo-inline.h"
+
+__attribute__ ((visibility ("hidden"))) int foo(int a, int b) { return a + b; }
+__attribute__ ((visibility ("default"))) int foo_default_visi(int a, int b) { return a + b; }
+
+
+__attribute__ ((visibility ("default"))) int fvih_1(int a, int b) { return a + fvih(); }
+
+int dataA = 34;
+
+namespace baz {
+  template 
+  T add(T a, T b) {
+return a + b;
+  }
+}
+
+namespace n {
+  template 
+  struct __attribute__((__visibility__("default"))) S {
+S() = default;
+~S() = default;
+int __attribute__((__visibility__(("default" func() const { return 32; }
+int __attribute__((__visibility__(("hidden" operator()() const { return 53; }
+  };
+}
+
+template  T neverUsed(T t) { return t + 2; }
+
+template<> int neverUsed(int t);
+
+void g() { n::S()(); }
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+}
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/IFSO/foo-inline.h
===
--- /dev/null
+++ clang/test/IFSO/foo-inline.h
@@ -0,0 +1,6 @@
+
+inline int fvih() {
+static int fortytwo = 42;
+  return fortytwo;
+}
+
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -64,6 +64,7 @@
   case GenerateHeaderModule:
 return llvm::make_unique();
   case GeneratePCH:return llvm::make_unique();
+  case GenerateIFSO:   return llvm::make_unique();
   case InitOnly:   return llvm::make_unique();
   case ParseSyntaxOnly:return llvm::make_unique();
   case ModuleFileInfo: return llvm::make_unique();
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -25,6 +25,8 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Index/CodegenNameGenerator.h"
 #include 
 #include 
 
@@ -158,6 +160,186 @@
   return true;
 }
 
+class IFSOFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;
+  StringRef InFile = "";
+  std::set ParsedTemplates;
+
+  enum RootDeclOrigin { TopLevel = 0, FromTU = 1, IsLate = 2 };
+
+  template 
+  bool WriteNamedDecl(const NamedDecl *ND,
+  std::vector &MangledNames, int RDO) {
+if (!isa(ND))
+  return false;
+if (ND->getVisibility() != DefaultVisibility)
+  return true;
+// If this is a FunctionDecl that is dependent on a template parameter, then
+// don't get the symbol because we can only export specializations.
+bool IsRDOLate = (RDO & IsLate);
+if (const auto *FD = dyn_cast(ND))
+  if (FD->isDependentContext() && !IsRDOLate)
+return true;
+index::CodegenNameGenerator CGNameGen(ND->getASTContext());
+std::string MangledName = CGNameGen.getName(ND);
+MangledNames.push_back(MangledName);
+// For now, lets just dump the -fdelayed-template-parsing decls until we
+// decide how to handle them.
+if (IsRDOLate) {
+  llvm::errs() << "LATE DECL:\n";
+  ND->dump();
+}
+return true;
+  }
+
+  template 
+  bool HandleSomeDecl(const NamedDecl *ND,
+  std::vector &MangledNames, int RDO) {
+if (!isa(ND))
+  return false;
+for (auto *I : cast(ND)->decls())
+  HandleNamedDecl(dyn_cast(I), Mangled

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1474994 , @compnerd wrote:

> Note that I am not advocating that this change go in as is - I think that 
> this is far from something which would be useful, and until it can produce 
> something meaningful, this shouldn't really be merged (i.e. generate 
> something which obj2yaml or some other tool can consume to generate the 
> barest ELF stub).


Neither was I, as I did not have an output format (but I've added something 
now). I just want feedback to iterate on.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

BTW, to clarify how this is intended to be used: the collection of the public 
interfaces defines the *public* interfaces of the module.  This information can 
be used to generate a **minimally** viable ELF module that the linker can use 
to link against - no `.text` segment or any other metadata (IIRC, you just need 
`.symtab`, `.shstrtab`, `.strtab`, `.dynsym` and the ELF header, but that is 
off the top of my head, and may be incorrect).


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1474777 , @jakehehrlich 
wrote:

> In D60974#1474751 , @plotfi wrote:
>
> > There are a few that I have in mind.
> >
> > 1. -emit-ifso could be added to a build to produce .so files as part of a 
> > device SDK (where we don't want to ship the runnable bits in the SDK, we 
> > ship those on the device updates).
>
>
> This would require creating a linker for these files. Is this what you intend 
> to do?


No, I do not intend to write any sort of linker in the classical sense. At most 
I'd be handling and merging symbol names from yaml files or possibly going from 
yaml -> elf .o -> .so. Weighing options.

>> 2. -emit-ifso could be added to whatever the existing set of build flags are 
>> to be run as a pre-build step to produce tapi-like .so files that break up 
>> build dependencies.
> 
> How would this work? To be clear, this would happen at the same time that .o 
> files are produced correct? The build graph would basically look the same but 
> your new linking step would replace the original link step and there would be 
> a pendent node for the original link step. This would not break up the build 
> graph much at all save for the fact that we might imagine that this new link 
> step would be much faster.

I was thinking that since -emit-ifso doesn't invoke the backend, it could be 
run as a pre-build step. Then during the build, linking is done against the 
ifso stubs that are already generated instead of the actual .so files.

> 
> 
>> 3. -emit-ifso -fvisibility=hidden could added to the invocation to disallow 
>> usage of non-public apis.
> 
> Can you be more specific? This is already how the actual modules do this 
> today so what is the advantage here.

I believe modules produce much more data than what I am doing here. What I am 
doing is trying to use visibility semantics to produce the bare minimum ELF and 
bare minimum symbols while still being able to link with the resulting .so file.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/lib/Frontend/FrontendActions.cpp:326
+(*OS) << "  - Name:.text\n";
+(*OS) << "Type:STT_SECTION\n";
+(*OS) << "Section: .text\n";

This is wrong, this marks the type to be a section symbol, which is not correct.



Comment at: clang/lib/Frontend/FrontendActions.cpp:332
+<< "Section: .text\n"
+<< "Binding: STB_GLOBAL\n";
+(*OS) << "...\n";

Hmm, you need to classify the data symbols properly and mark them 
appropriately, not as `STT_FUNC`.  This is particularly important on ARM where 
the ISA selection bit may alter the address.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

Why is this better than producing output from the output of the static linker 
at the end? Also how do we plan on integrating this with llvm-elfabi so that we 
don't have unnecessarily duplicated efforts? Why does that tool not suffice?

> I'm well versed in the complexities of a linker - I've worked extensively on 
> the GNU linkers as well as with lld. The visibility of the symbols is 
> actually computed by the compiler - the STV_* flags associated with the 
> symtab entry give you that information which is actually tracked through the 
> frontend to the backend. Yes, each linker behaves differently - including lld 
> which completely changes the semantics of Unix/ELF linking guarantees. In 
> fact every single attribute that you mentioned is something which is emitted 
> from the compiler - which we have access to here given that we are in clang 
> itself. I think that this can be made to work properly, though will probably 
> require some iteration to get all the corner cases, and that you may be 
> slightly dismissive of this approach.
> 
> Ah, okay, I didn't pick up that you were using module as the actual module 
> from the context since normally the module doesn't control what it exposes 
> itself.
> 
> Note that I am not advocating that this change go in as is - I think that 
> this is far from something which would be useful, and until it can produce 
> something meaningful, this shouldn't really be merged (i.e. generate 
> something which obj2yaml or some other tool can consume to generate the 
> barest ELF stub).

It's not a matter of where they're produced, its how you treat them and ensure 
that they're correctly handled so that the final produced stub has the correct 
output. I don't think this is entirely trivial when you consider all of those 
different aspects that I listed.

> No, I do not intend to write any sort of linker in the classical sense. At 
> most I'd be handling and merging symbol names from yaml files or possibly 
> going from yaml -> elf .o -> .so. Weighing options.

I'm calling that a kind of linker here. I'm not saying it would be as 
complicated as a full linker but it would be a linker. I am claiming it would 
be more complicated than the dwp linker however. Whatever you're calling it you 
have to write it and that's an important part of this proposal process but that 
wasn't mentioned until I asked about it.

> I was thinking that since -emit-ifso doesn't invoke the backend, it could be 
> run as a pre-build step. Then during the build, linking is done against the 
> ifso stubs that are already generated instead of the actual .so files.

That seems a lot better than what I understood. This behavior might be useful 
to some people. In particular you don't need .tbe files in your source tree so 
if that isn't something you want to have this would be a less ideal alternative 
because it was to invoke enough of the compiler to get rich symbol information 
but not so much that its as slow as compiling. You pay for this by having to 
reparse and go though some of the compilation process and you have to 
link/merge many files to produce the stub but it doesn't require funny files in 
your tree. I think that's a niche that some people might want. The tradeoff 
between not having to deal with .tbe files and the huge increase of build 
steps, computation needed (overall build times could very well decrease if your 
build wasn't sufficiently parallel due to linking orders), and total amount of 
IO seems like a tradeoff that I personally wouldn't want to make. Is this a 
trade off you want to make? Almost every metric I can think of would be worse 
than just adding .tbe files into your source code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1476084 , @jakehehrlich 
wrote:

> Why is this better than producing output from the output of the static linker 
> at the end? Also how do we plan on integrating this with llvm-elfabi so that 
> we don't have unnecessarily duplicated efforts? Why does that tool not 
> suffice?
>
> > I'm well versed in the complexities of a linker - I've worked extensively 
> > on the GNU linkers as well as with lld. The visibility of the symbols is 
> > actually computed by the compiler - the STV_* flags associated with the 
> > symtab entry give you that information which is actually tracked through 
> > the frontend to the backend. Yes, each linker behaves differently - 
> > including lld which completely changes the semantics of Unix/ELF linking 
> > guarantees. In fact every single attribute that you mentioned is something 
> > which is emitted from the compiler - which we have access to here given 
> > that we are in clang itself. I think that this can be made to work 
> > properly, though will probably require some iteration to get all the corner 
> > cases, and that you may be slightly dismissive of this approach.
> > 
> > Ah, okay, I didn't pick up that you were using module as the actual module 
> > from the context since normally the module doesn't control what it exposes 
> > itself.
> > 
> > Note that I am not advocating that this change go in as is - I think that 
> > this is far from something which would be useful, and until it can produce 
> > something meaningful, this shouldn't really be merged (i.e. generate 
> > something which obj2yaml or some other tool can consume to generate the 
> > barest ELF stub).
>
> It's not a matter of where they're produced, its how you treat them and 
> ensure that they're correctly handled so that the final produced stub has the 
> correct output. I don't think this is entirely trivial when you consider all 
> of those different aspects that I listed.


I'll let @compnerd respond to the above...

>> No, I do not intend to write any sort of linker in the classical sense. At 
>> most I'd be handling and merging symbol names from yaml files or possibly 
>> going from yaml -> elf .o -> .so. Weighing options.
> 
> I'm calling that a kind of linker here. I'm not saying it would be as 
> complicated as a full linker but it would be a linker. I am claiming it would 
> be more complicated than the dwp linker however. Whatever you're calling it 
> you have to write it and that's an important part of this proposal process 
> but that wasn't mentioned until I asked about it.

You are right about this, because the original proposal was proposing a header 
-> .so file processor. But because of the issue of faithfully reproducing the 
ifso based on the build flags, I decided to re-implement things using the clang 
driver which results the need to do a symbol merge. I could do another proposal 
RFC if you think that is necessary.

>> I was thinking that since -emit-ifso doesn't invoke the backend, it could be 
>> run as a pre-build step. Then during the build, linking is done against the 
>> ifso stubs that are already generated instead of the actual .so files.
> 
> That seems a lot better than what I understood. This behavior might be useful 
> to some people. In particular you don't need .tbe files in your source tree 
> so if that isn't something you want to have this would be a less ideal 
> alternative because it was to invoke enough of the compiler to get rich 
> symbol information but not so much that its as slow as compiling. You pay for 
> this by having to reparse and go though some of the compilation process and 
> you have to link/merge many files to produce the stub but it doesn't require 
> funny files in your tree. I think that's a niche that some people might want. 
> The tradeoff between not having to deal with .tbe files and the huge increase 
> of build steps, computation needed (overall build times could very well 
> decrease if your build wasn't sufficiently parallel due to linking orders), 
> and total amount of IO seems like a tradeoff that I personally wouldn't want 
> to make. Is this a trade off you want to make? Almost every metric I can 
> think of would be worse than just adding .tbe files into your source code.

I actually don't see why something like this couldn't be used in both scenarios 
(ie some users generating the files at build time, and others using -emit-ifso 
to periodically generate and land changes to checked in files).


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

> I actually don't see why something like this couldn't be used in both 
> scenarios (ie some users generating the files at build time, and others using 
> -emit-ifso to periodically generate and land changes to checked in files).

They could be used in parallel in most cases but they would likely have 
divergent behavior unless we shared the writers and there be duplicated code. 
This is not strictly a bad thing and efforts to merge them might not be 
fruitful or ideal but its something I think we should discuss. A real 
possibility is that llvm-elfabi could be the linker than merges these since it 
already has a carefully thought out and evolving internal representation of an 
ELF file and will soon have readers/writers for both stubs and .tbe files.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

The strong motivation here is to avoid having the list be maintained outside of 
the source code.  I am not too strongly tied to the YAML approach as long as 
the binaries that we get at the end are equivalent to the pruned ELF binaries 
from the YAML2ELF approach.  However, the trade off here is valuable - it 
allows for the source code to be the source of truth for the interface set.  
The intermediate emission is due to the fact that this requires program level 
visibility and having the intermediates an extra step to merge is relatively 
simpler.  In fact, you could emit the TBE files from this and check that into 
the tree if you really wanted (but allowing the source to remain the source of 
truth I think is a valid and valuable trade off).  The reason for the selection 
of YAML is simply pragmatism - that is already available in the tree.  If the 
TBE linker is available now, I have no issue with that being the intermediate 
representation (assuming that the representation is easy to parse 
programatically).


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

> The strong motivation here is to avoid having the list be maintained outside 
> of the source code. I am not too strongly tied to the YAML approach as long 
> as the binaries that we get at the end are equivalent to the pruned ELF 
> binaries from the YAML2ELF approach. However, the trade off here is valuable 
> - it allows for the source code to be the source of truth for the interface 
> set. The intermediate emission is due to the fact that this requires program 
> level visibility and having the intermediates an extra step to merge is 
> relatively simpler. In fact, you could emit the TBE files from this and check 
> that into the tree if you really wanted (but allowing the source to remain 
> the source of truth I think is a valid and valuable trade off). The reason 
> for the selection of YAML is simply pragmatism - that is already available in 
> the tree. If the TBE linker is available now, I have no issue with that being 
> the intermediate representation (assuming that the representation is easy to 
> parse programatically).

You can verify that a .tbe file is consistent with the finally fully linked 
executable to ensure that it is consistent with the actual source of truth, the 
finally linked binary (the source code is not really the source of truth on 
this since compiler and linker flags alter these things). This is a planned 
feature of llvm-elfabi. Keep in mind that having .tbe files in tree means that 
you can also review the public interface of modules exactly as a part of code 
review which your method allows for only in the traditional way. When you 
consider that compiler and linker flags can change these things there is no one 
single source of truth that humans can easily look at without something like 
the .tbe files. This was a major motivation behind the choice here.

The tbe format is not suitable as a pre-merged format I think. I propose that 
you could take your pre-merged format and do the merging in llvm-elfabi (or 
rather, the tool would be a symlink to the same binary with a different name) 
to produce the same internal representation that llvm-elfabi already uses and 
then you could use the same writers for both .tbe and ELF stubs. This would 
allow sharing of the relevant code and would trivially allow both .tbe files 
and stubs to be produced. Also, as a side note, .tbe files are also YAML for 
the same reason you're using it here. I don't have an issue with that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Okay, if the TBE isn't suitable for the pre-merged format, then I think that 
this should be okay with the current approach.  This is more about the 
generation and not wanting to have to maintain a separate side table of 
information manually.  It just is a different trade off.  But, sounds like we 
are converging towards a solution that can be used to address the various 
tradeoff points that seem to matter.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

Aside from comments that @jakehehrlich already made which I agree with, I'm 
also concerned about the use of `yaml2obj`. AFAIK that tool has always been 
intended as a testing, not a production tool, but with this change this would 
no longer be the case. Specifically, it formalizes the `yaml2obj` input format 
by making it one of the output formats supported by Clang. I believe that this 
is such a fundamental change that it should be discussed separately before 
moving on with this implementation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 196516.
plotfi added a comment.

Updated to support properly setting OBJECT/FUNC types as well as section flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/IFSO/foo-inline.h
  clang/test/IFSO/foo.cpp

Index: clang/test/IFSO/foo.cpp
===
--- /dev/null
+++ clang/test/IFSO/foo.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang -emit-ifso -fvisibility=hidden %s -o - | FileCheck --check-prefix=CHECK-HIDDEN %s
+// RUN: %clang -emit-ifso %s -o - | FileCheck %s
+
+// CHECK-HIDDEN-NOT: __Z4fbarff
+// CHECK: __Z4fbarff
+
+
+
+
+// CHECK-HIDDEN-NOT: __Z3fooii
+// CHECK-NOT:__Z3fooii
+
+
+
+#include "foo-inline.h"
+
+__attribute__ ((visibility ("hidden"))) int foo(int a, int b) { return a + b; }
+__attribute__ ((visibility ("default"))) int foo_default_visi(int a, int b) { return a + b; }
+
+
+__attribute__ ((visibility ("default"))) int fvih_1(int a, int b) { return a + fvih(); }
+
+int dataA = 34;
+
+namespace baz {
+  template 
+  T add(T a, T b) {
+return a + b;
+  }
+}
+
+namespace n {
+  template 
+  struct __attribute__((__visibility__("default"))) S {
+S() = default;
+~S() = default;
+int __attribute__((__visibility__(("default" func() const { return 32; }
+int __attribute__((__visibility__(("hidden" operator()() const { return 53; }
+  };
+}
+
+template  T neverUsed(T t) { return t + 2; }
+
+template<> int neverUsed(int t);
+
+void g() { n::S()(); }
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+}
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/IFSO/foo-inline.h
===
--- /dev/null
+++ clang/test/IFSO/foo-inline.h
@@ -0,0 +1,6 @@
+
+inline int fvih() {
+static int fortytwo = 42;
+  return fortytwo;
+}
+
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -64,6 +64,7 @@
   case GenerateHeaderModule:
 return llvm::make_unique();
   case GeneratePCH:return llvm::make_unique();
+  case GenerateIFSO:   return llvm::make_unique();
   case InitOnly:   return llvm::make_unique();
   case ParseSyntaxOnly:return llvm::make_unique();
   case ModuleFileInfo: return llvm::make_unique();
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -20,11 +20,14 @@
 #include "clang/Sema/TemplateInstCallback.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ASTWriter.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Index/CodegenNameGenerator.h"
 #include 
 #include 
 
@@ -158,6 +161,260 @@
   return true;
 }
 
+class IFSOFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;
+  StringRef InFile = "";
+  std::set ParsedTemplates;
+
+  enum RootDeclOrigin { TopLevel = 0, FromTU = 1, IsLate = 2 };
+  struct MangledSymbol {
+std::string Name = "";
+uint8_t Type = llvm::ELF::STT_NOTYPE;
+uint8_t Binding = llvm::ELF::STB_LOCAL;
+MangledSymbol(const std::string &Name, uint8_t Type, uint8_t Binding)
+: Name(Name), Type(Type), Binding(Binding) {}
+  };
+  using MangledSymbols = std::map;
+
+  template 
+  bool WriteNamedDecl(const NamedDecl *ND, MangledSymbols &Symbols, int RDO) {
+if (!isa(ND))
+  return false;
+if (Symbols.find(ND) != Symbols.end())
+  return true;
+if (isa(ND))
+  return true;
+if (ND->getVisibility() != DefaultVisibility)
+  return true;
+// If this is a FunctionDecl that is dependent on a template parameter, then
+// don't get the symbol because we can only export specializations.
+bool IsRDOLate = (RDO & IsLate);
+if (const auto *FD = dyn_cast(ND))
+  if (FD->isDependentContext() && !IsRDOLate)
+return true;
+index::CodegenNameGenerator CGNameGen(ND->getASTContext());
+std::string MangledName = CGName

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1477302 , @phosek wrote:

> Aside from comments that @jakehehrlich already made which I agree with, I'm 
> also concerned about the use of `yaml2obj`. AFAIK that tool has always been 
> intended as a testing, not a production tool, but with this change this would 
> no longer be the case. Specifically, it formalizes the `yaml2obj` input 
> format by making it one of the output formats supported by Clang. I believe 
> that this is such a fundamental change that it should be discussed separately 
> before moving on with this implementation.


My main goal here is to emit some kind of a mergable format. If not yaml or 
tbe, then directly emitting the ELF. yaml is a stepping stone for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

To be clear I'm only really giving my thumbs up to a specific high level plan 
that I believe handles a case that llvm-elfabi wouldn't otherwise handle on its 
own. I consider the specifics like the output format still in the air right 
now. I strongly believe that a unique yaml or json format should be used for 
this. We should discuss the specifics of what should go in that format and have 
a discussion on that format. The trick is to capture everything in .o files 
that could wind up mattering to a .tbe file and nothing more. I haven't thought 
though what that entails completely yet but I'll think about it and throw up my 
proposal. Once we have at least one proposal we can start iterating on the 
specifics of it.

Here's the plan I think we can converge which at least covers all of my 
concerns:

1. A flag is added to clang that would cause clang to emit a pre-merged format 
and not invoke the backend
2. The pre-merged format can then be merged into a stub or .tbe file as part of 
the same binary (but an effectively different tool since a symlink and options 
parser will be used instead) as llvm-elfabi
3. The yaml code for this format will go in llvm so that both clang and the new 
merging tool could use it
4. We need to discuss the specifics of the pre-merged format to ensure that it 
captures everything that matters (and hopefully nothing more)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

@jakehehrlich - when do you expect to have your idea put up?  I don't think 
that it is fair to have this wait until you have time to put something up that 
can be discussed.  I think that getting this working and then iterating on it 
and migrating it over to some shared representation is something which we could 
do - that tends to be a common thing that I have seen happen multiple times 
with the necessary work never materialising.  Re-use of the YAML structure 
means that we can iterate and identify the pieces that are necessary, though, I 
expect that largely, what will be needed is the name, the binding, the 
visibility, possibly the size (for TBEs), the section, and the type, at least 
for anything which adheres to the GABI.  If you have extensions outside of 
GABI, this will need to be adjusted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

@phosek - I completely agree, I really would prefer that this not promote the 
`yaml2obj` tool to an officially supported tool.  The reason for using the same 
output format is for testing convenience, and this should not really invoke the 
tool (this should work without a `yaml2obj` or `obj2yaml` available on the 
system).  It is similar to the idea of the IR - just a serialization format for 
testing/debugging/inspecting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

In D60974#1477690 , @compnerd wrote:

> @jakehehrlich - when do you expect to have your idea put up?  I don't think 
> that it is fair to have this wait until you have time to put something up 
> that can be discussed.  I think that getting this working and then iterating 
> on it and migrating it over to some shared representation is something which 
> we could do - that tends to be a common thing that I have seen happen 
> multiple times with the necessary work never materialising.  Re-use of the 
> YAML structure means that we can iterate and identify the pieces that are 
> necessary, though, I expect that largely, what will be needed is the name, 
> the binding, the visibility, possibly the size (for TBEs), the section, and 
> the type, at least for anything which adheres to the GABI.  If you have 
> extensions outside of GABI, this will need to be adjusted.


I don't know when but in the next 2 days likely. Regardless of my timeline I 
don't think I'm blocking anyone. I'm just saying that I will put up a proposal. 
You should also put up a proposal as well. The yaml2obj format is just not well 
designed for any purpose but is far from designed for this purpose. yaml2obj 
works ok-ish for testing. I'd rather start with a minimal format and then add 
things to it rather than starting with a bloated and ill designed format and 
then create a new format from that experience. Creating a minimal format 
shouldn't be hard and I suspect our proposal will look extremely similar; I'd 
wager if you put up a proposal I'd probably just review your proposal rather 
than bother writing out my own.

As for what I think it would entail. I think name, weather or not the symbol is 
defined or undefined, visibility, size, alignment (this is a feature of the 
section and the symbol offset) and type will all mater but not all combinations 
make sense. Sections don't matter as it turns out but alignment does for copy 
relocations. When we started llvm-elfabi I thought at least the section 
permissions mattered but they don't really. While .tbe has things like soname 
and dt_needed that isn't needed here.  The top of the file should probably 
contain the architecture. Other details in the ELF header shouldn't be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 196535.
plotfi added a comment.

First stab at trying to properly handle Weak Symbols


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/IFSO/foo-inline.h
  clang/test/IFSO/foo.cpp

Index: clang/test/IFSO/foo.cpp
===
--- /dev/null
+++ clang/test/IFSO/foo.cpp
@@ -0,0 +1,56 @@
+// RUN: %clang -emit-ifso -fvisibility=hidden %s -o - | FileCheck --check-prefix=CHECK-HIDDEN %s
+// RUN: %clang -emit-ifso %s -o - | FileCheck %s
+
+// CHECK-HIDDEN-NOT: __Z4fbarff
+// CHECK: __Z4fbarff
+
+
+
+
+// CHECK-HIDDEN-NOT: __Z3fooii
+// CHECK-NOT:__Z3fooii
+
+
+
+#include "foo-inline.h"
+
+__attribute__ ((visibility ("hidden"))) int foo(int a, int b) { return a + b; }
+__attribute__ ((visibility ("default"))) int foo_default_visi(int a, int b) { return a + b; }
+
+
+__attribute__ ((visibility ("default"))) int fvih_1(int a, int b) { return a + fvih(); }
+
+
+__attribute__((weak)) int someWeakFunc() { return 42; }
+
+int dataA = 34;
+
+namespace baz {
+  template 
+  T add(T a, T b) {
+return a + b;
+  }
+}
+
+namespace n {
+  template 
+  struct __attribute__((__visibility__("default"))) S {
+S() = default;
+~S() = default;
+int __attribute__((__visibility__(("default" func() const { return 32; }
+int __attribute__((__visibility__(("hidden" operator()() const { return 53; }
+  };
+}
+
+template  T neverUsed(T t) { return t + 2; }
+
+template<> int neverUsed(int t);
+
+void g() { n::S()(); }
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+}
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/IFSO/foo-inline.h
===
--- /dev/null
+++ clang/test/IFSO/foo-inline.h
@@ -0,0 +1,6 @@
+
+inline int fvih() {
+static int fortytwo = 42;
+  return fortytwo;
+}
+
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -64,6 +64,7 @@
   case GenerateHeaderModule:
 return llvm::make_unique();
   case GeneratePCH:return llvm::make_unique();
+  case GenerateIFSO:   return llvm::make_unique();
   case InitOnly:   return llvm::make_unique();
   case ParseSyntaxOnly:return llvm::make_unique();
   case ModuleFileInfo: return llvm::make_unique();
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -20,11 +20,14 @@
 #include "clang/Sema/TemplateInstCallback.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ASTWriter.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Index/CodegenNameGenerator.h"
 #include 
 #include 
 
@@ -158,6 +161,256 @@
   return true;
 }
 
+class IFSOFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;
+  StringRef InFile = "";
+  std::set ParsedTemplates;
+
+  enum RootDeclOrigin { TopLevel = 0, FromTU = 1, IsLate = 2 };
+  struct MangledSymbol {
+std::string Name = "";
+uint8_t Type = llvm::ELF::STT_NOTYPE;
+uint8_t Binding = llvm::ELF::STB_LOCAL;
+MangledSymbol(const std::string &Name, uint8_t Type, uint8_t Binding)
+: Name(Name), Type(Type), Binding(Binding) {}
+  };
+  using MangledSymbols = std::map;
+
+  template 
+  bool WriteNamedDecl(const NamedDecl *ND, MangledSymbols &Symbols, int RDO) {
+if (!isa(ND))
+  return false;
+if (Symbols.find(ND) != Symbols.end())
+  return true;
+if (isa(ND))
+  return true;
+if (ND->getVisibility() != DefaultVisibility)
+  return true;
+// If this is a FunctionDecl that is dependent on a template parameter, then
+// don't get the symbol because we can only export specializations.
+bool IsRDOLate = (RDO & IsLate);
+if (const auto *FD = dyn_cast(ND))
+  if (FD->isDependentContext() && !IsRDOLate)
+return true;
+index::CodegenNameGenerator CGNameGen(ND->getASTContext());
+  

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1477761 , @jakehehrlich 
wrote:

> In D60974#1477690 , @compnerd wrote:
>
> > @jakehehrlich - when do you expect to have your idea put up?  I don't think 
> > that it is fair to have this wait until you have time to put something up 
> > that can be discussed.  I think that getting this working and then 
> > iterating on it and migrating it over to some shared representation is 
> > something which we could do - that tends to be a common thing that I have 
> > seen happen multiple times with the necessary work never materialising.  
> > Re-use of the YAML structure means that we can iterate and identify the 
> > pieces that are necessary, though, I expect that largely, what will be 
> > needed is the name, the binding, the visibility, possibly the size (for 
> > TBEs), the section, and the type, at least for anything which adheres to 
> > the GABI.  If you have extensions outside of GABI, this will need to be 
> > adjusted.
>
>
> I don't know when but in the next 2 days likely. Regardless of my timeline I 
> don't think I'm blocking anyone. I'm just saying that I will put up a 
> proposal. You should also put up a proposal as well. The yaml2obj format is 
> just not well designed for any purpose but is far from designed for this 
> purpose. yaml2obj works ok-ish for testing. I'd rather start with a minimal 
> format and then add things to it rather than starting with a bloated and ill 
> designed format and then create a new format from that experience. Creating a 
> minimal format shouldn't be hard and I suspect our proposal will look 
> extremely similar; I'd wager if you put up a proposal I'd probably just 
> review your proposal rather than bother writing out my own.
>
> As for what I think it would entail. I think name, weather or not the symbol 
> is defined or undefined, visibility, size, alignment (this is a feature of 
> the section and the symbol offset) and type will all mater but not all 
> combinations make sense. Sections don't matter as it turns out but alignment 
> does for copy relocations. When we started llvm-elfabi I thought at least the 
> section permissions mattered but they don't really. While .tbe has things 
> like soname and dt_needed that isn't needed here.  The top of the file should 
> probably contain the architecture. Other details in the ELF header shouldn't 
> be needed.


Would you be ok with having yaml emission as a debug/testing format (not as the 
official format, which I will post in a proposal this week or early next)? I 
suppose whatever tool handles the format can convert it to yaml, but it could 
still be convenient for debugging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 200393.
plotfi added a comment.

adding a nice inline test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.cpp
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-NEXT:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-NEXT:   - Name:_Z8weakFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_WEAK
+// CHECK-YAML-NEXT:   - Name:_Z10strongFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_GLOBAL
+
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() {}
+
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK: _Z10cmdVisiblev
+void cmdVisible() {}
+
Index: clang/test/InterfaceStubs/template-namespace-function.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/template-namespace-function.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _ZN3qux3barEii: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIiEET_S1_S1_: { Type: Func }
+// CHECK-NEXT:  _Z4fbarff: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIfEET_S1_S1_: { Type: Func }
+
+namespace baz {
+template 
+T add(T a, T b) {
+  return a + b;
+}
+} // namespace baz
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+} // namespace qux
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/InterfaceStubs/object.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/object.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// CHECK: data: { Type: Object, Size: 4 }
+int data = 1844;
+
Index: clang/test/InterfaceStubs/inline.h
===
--- /dev/null
+++ clang/test/InterfaceStubs/inline.h
@@ -0,0 +1,6 @@
+
+inline int fvih() {
+static 

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 200602.
plotfi added a comment.

fixing support for static functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.cpp
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-NEXT:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-NEXT:   - Name:_Z8weakFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_WEAK
+// CHECK-YAML-NEXT:   - Name:_Z10strongFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_GLOBAL
+
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() {}
+
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK: _Z10cmdVisiblev
+void cmdVisible() {}
+
Index: clang/test/InterfaceStubs/template-namespace-function.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/template-namespace-function.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _ZN3qux3barEii: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIiEET_S1_S1_: { Type: Func }
+// CHECK-NEXT:  _Z4fbarff: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIfEET_S1_S1_: { Type: Func }
+
+namespace baz {
+template 
+T add(T a, T b) {
+  return a + b;
+}
+} // namespace baz
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+} // namespace qux
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/InterfaceStubs/object.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/object.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// CHECK: data: { Type: Object, Size: 4 }
+int data = 1844;
+
Index: clang/test/InterfaceStubs/inline.h
===
--- /dev/null
+++ clang/test/InterfaceStubs/inline.h
@@ -0,0 +1,6 @@
+
+inline int fvih()

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 200805.
plotfi added a comment.

adding static/extern tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.cpp
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-NEXT:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-NEXT:   - Name:_Z8weakFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_WEAK
+// CHECK-YAML-NEXT:   - Name:_Z10strongFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_GLOBAL
+
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() {}
+
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK: _Z10cmdVisiblev
+void cmdVisible() {}
+
Index: clang/test/InterfaceStubs/template-namespace-function.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/template-namespace-function.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _ZN3qux3barEii: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIiEET_S1_S1_: { Type: Func }
+// CHECK-NEXT:  _Z4fbarff: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIfEET_S1_S1_: { Type: Func }
+
+namespace baz {
+template 
+T add(T a, T b) {
+  return a + b;
+}
+} // namespace baz
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+} // namespace qux
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/InterfaceStubs/object.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/object.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// CHECK: data: { Type: Object, Size: 4 }
+int data = 1844;
+
Index: clang/test/InterfaceStubs/inline.h
===
--- /dev/null
+++ clang/test/InterfaceStubs/inline.h
@@

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 200828.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-NEXT:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-NEXT:   - Name:_Z8weakFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_WEAK
+// CHECK-YAML-NEXT:   - Name:_Z10strongFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_GLOBAL
+
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() {}
+
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK: _Z10cmdVisiblev
+void cmdVisible() {}
+
Index: clang/test/InterfaceStubs/template-namespace-function.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/template-namespace-function.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _ZN3qux3barEii: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIiEET_S1_S1_: { Type: Func }
+// CHECK-NEXT:  _Z4fbarff: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIfEET_S1_S1_: { Type: Func }
+
+namespace baz {
+template 
+T add(T a, T b) {
+  return a + b;
+}
+} // namespace baz
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+} // namespace qux
+
+float fbar(float a, float b) { return baz::add(a, b); }
+
Index: clang/test/InterfaceStubs/object.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/object.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// CHECK: data: { Type: Object, Size: 4 }
+int data = 1844;
+
Index: clang/test/InterfaceStubs/inline.h
===
--- /dev/null
+++ clang/test/InterfaceStubs/inline.h
@@ -0,0 +1,4 @@
+INLINE int bar() {
+  static int var = 42;
+  return var;
+}
Index: clang/test

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 201104.
plotfi added a comment.

Improving visibility extraction for class template specializations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-NEXT:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-NEXT:   - Name:_Z8weakFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_WEAK
+// CHECK-YAML-NEXT:   - Name:_Z10strongFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_GLOBAL
+
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() {}
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK: _Z10cmdVisiblev
+void cmdVisible() {}
Index: clang/test/InterfaceStubs/template-namespace-function.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/template-namespace-function.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _ZN3qux3barEii: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIiEET_S1_S1_: { Type: Func }
+// CHECK-NEXT:  _Z4fbarff: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIfEET_S1_S1_: { Type: Func }
+
+namespace baz {
+template 
+T add(T a, T b) {
+  return a + b;
+}
+} // namespace baz
+
+namespace qux {
+int bar(int a, int b) { return baz::add(a, b); }
+} // namespace qux
+
+float fbar(float a, float b) { return baz::add(a, b); }
Index: clang/test/InterfaceStubs/object.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/object.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// CHECK: data: { Type: Object, Size: 4 }
+int data = 1844;
Index: clang/test/InterfaceStubs/inline.h
===
--- /dev/null
+++ clang/test/InterfaceStubs/inline.h
@@ -0,0 +

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 201339.
plotfi added a comment.

More test improvement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-NEXT:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-NEXT:   - Name:_Z8weakFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_WEAK
+// CHECK-YAML-NEXT:   - Name:_Z10strongFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS: _Z10strongFuncv
+// CHECK-SYMBOLS: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/template-namespace-function.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/template-namespace-function.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _ZN3qux3barEii: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIiEET_S1_S1_: { Type: Func }
+// CHECK-NEXT:  _Z4fbarff: { Type: Func }
+// CHECK-NEXT:  _ZN3baz3addIfEET_S1_S1_: { Type: Func }
+
+// Same symbols just different order.
+// CHECK-SYMBOLS:   _Z4fbarff
+// CHECK-SYMBOLS-NEXT:  _ZN3baz3addIfEET_S1_S1_
+// CHECK-SYMBOLS-NEXT:  _ZN3baz3addIiEET_S1_S1_
+//

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 201471.
plotfi added a comment.

virtual function test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-NEXT:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-NEXT:   - Name:_Z8weakFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_WEAK
+// CHECK-YAML-NEXT:   - Name:_Z10strongFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS: _Z10strongFuncv
+// CHECK-SYMBOLS: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// CHECK-TAPI-NOT: _ZNK1Q5func1Ev
+// CHECK-TAPI-NOT: _ZNK1Q5func2Ev
+// CHECK-SYMBOLS: NOTYPE  GLOBAL HIDDEN   {{.*}} _ZNK1Q5func1Ev
+// CHECK-SYMBOLS: NOTYPE  GLOBAL DEFAULT  {{.*}} _ZNK1Q5func2Ev
+struct Q {
+  virtual HIDDEN  int func1() cons

[PATCH] D60974: Clang IFSO driver action.

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added inline comments.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:30
+uint8_t Binding;
+MangledSymbol() = delete;
+MangledSymbol(const std::string &Name, const std::string &ParentName,

would be nice to have a newline before the ctor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 202976.
plotfi added a comment.

Adding a test for inheritance and constructors/destroctors. Also addressing 
feedback from @alexshap


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/public-hidden.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-NEXT:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-NEXT:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-NEXT:   - Name:_Z8weakFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_WEAK
+// CHECK-YAML-NEXT:   - Name:_Z10strongFuncv
+// CHECK-YAML-NEXT: Type:STT_FUNC
+// CHECK-YAML-NEXT: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS: _Z10strongFuncv
+// CHECK-SYMBOLS: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// CHECK-TAPI-NOT: _ZNK1Q5func1Ev
+// CHECK-TAPI-NOT: _ZNK1Q5func2Ev
+// CHECK-SYMBOLS: NOTYPE  GLOBAL HIDDEN   {{.*}} _Z

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 203464.
plotfi added a comment.

Updated hidden parent/child class inheritance cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/public-hidden.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// CHECK-TAPI-NOT: _ZNK1Q5func1Ev
+// CHECK-TAPI-NOT: _ZNK1Q5func2Ev
+// CHECK-SYMBOLS-DAG: NOTYPE  GLOBAL HIDDEN   {{.*}} _ZNK1Q5func1Ev
+// CHECK-S

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 203465.
plotfi added a comment.

git mv'ed hidden-class-inheritance.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// CHECK-TAPI-NOT: _ZNK1Q5func1Ev
+// CHECK-TAPI-NOT: _ZNK1Q5func2Ev
+// CHECK-SYMBOLS-DAG: NOTYPE  GLOBAL HIDDEN   {{.*}} _ZNK1Q5func1Ev
+// CHECK-SYMB

[PATCH] D60974: Clang IFSO driver action.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a reviewer: rupprecht.
rupprecht added a comment.

Can you upload this patch with context? Either use arc or upload w/ -U9

I seem to have a lot of comments, but they're all nits -- my major concern 
being the `llvm_unreachable`s should be errors instead (i.e. should be 
triggered in release builds).

Since this is clearly labeled as experimental, I think you should feel free to 
commit if you can get another lgtm (@compnerd?)




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3614-3616
+  Args.hasArg(options::OPT_iterface_stub_version_EQ)
+  ? Args.getLastArgValue(options::OPT_iterface_stub_version_EQ)
+  : "")

`Args.getLastArgValue(options::OPT_iterface_stub_version_EQ)` should already 
default to returning the empty string if unset (note the default param)



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1678-1680
+  Args.hasArg(OPT_iterface_stub_version_EQ)
+  ? Args.getLastArgValue(OPT_iterface_stub_version_EQ)
+  : "")

(same here)



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1688
+  if (!ProgramActionPair.second)
+llvm_unreachable("Must specify a valid interface stub format.");
+  Opts.ProgramAction = ProgramActionPair.first;

I think this is very much reachable if someone provides 
`--interface-stub-version=x`



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:20-21
+  CompilerInstance &Instance;
+  StringRef InFile = "";
+  StringRef Format = "";
+  std::set ParsedTemplates;

nit: these default initializations are redundant



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:26
+  struct MangledSymbol {
+std::string ParentName = "";
+uint8_t Type;

ditto



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:39-41
+if (!(RDO & FromTU))
+  return true;
+if (Symbols.find(ND) != Symbols.end())

Can you add a comment why these two are excluded?



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:41
+  return true;
+if (Symbols.find(ND) != Symbols.end())
+  return true;

llvm::is_contained(Symbols, ND)



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:44-47
+if (isa(ND))
+  return true;
+if (isa(ND))
+  return true;

This would be terser (and more readable) combined; `if (isa(ND) || 
isa(ND))`



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:95
+  index::CodegenNameGenerator CGNameGen(ND->getASTContext());
+  auto MangledNames = CGNameGen.getAllManglings(ND);
+  if (MangledNames.size() == 1)

auto -> std::vector, unless the return type is obvious to anyone 
that commits here (i.e. not me)



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:107
+  index::CodegenNameGenerator CGNameGen(ND->getASTContext());
+  auto MangledName = CGNameGen.getName(ND);
+  auto MangledNames = CGNameGen.getAllManglings(ND);

auto -> std::string



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:206-207
+// catch anything that's not a VarDecl or Template/FunctionDecl.
+llvm_unreachable("clang -emit-iterface-stubs: Expected a function or "
+ "function template decl.");
+return false;

This should be an error, not llvm_unreachable (which I think gets filtered out 
in release builds)



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:242
+MangledSymbols Symbols;
+auto OS = Instance.createDefaultOutputFile(/*Binary=*/false, InFile, 
"ifo");
+if (!OS)

elsewhere it looks like this was changed to "ifs"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 203638.
plotfi added a comment.

-U99


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// CHECK-TAPI-NOT: _ZNK1Q5func1Ev
+// CHECK-TAPI-NOT: _ZNK1Q5func2Ev
+// CHECK-SYMBOLS-DAG: NOTYPE  GLOBAL HIDDEN   {{.*}} _ZNK1Q5func1Ev
+// CHECK-SYMBOLS-DAG: NOTYPE  GLOBAL DEFAUL

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

There seems to be some confusion about the term "format" here. There's the YAML 
format but within that there are specific YAML formats. My proposal will be to 
have a specific YAML format that just isn't the one used by yaml2obj. YAML 
isn't the issue, it's the structure that that tool specifically uses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

As an example, .tbe and .tbd files use YAML but in a very specific well defined 
structure. yaml2obj also uses a specific format which is actually well defined, 
it just doesn't map on very well here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1479467 , @jakehehrlich 
wrote:

> As an example, .tbe and .tbd files use YAML but in a very specific well 
> defined structure. yaml2obj also uses a specific format which is actually 
> well defined, it just doesn't map on very well here.


I specifically meant the yaml2obj format. Mainly because yaml2obj is a handy 
tool for developing things. I suppose one way to do it could be to put together 
a more stable format and output the yaml2obj format for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

Why would you convert the actual format, which is already using YAML, to the 
yaml2obj format? That seems to have zero gain to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

So, @compnerd and I have discussed and we'd like to propose a yaml schema that 
can express what we need for ifsos for the Sections and the Symbols. Something 
like:

  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

So, @compnerd and I have discussed and we'd like to propose a yaml schema that 
can express what we need for ifsos for the Sections and the Symbols.

For the Symbols we want:

  Symbols:
- Name:_dataA
  Type:STT_OBJECT
  Section: .data
  Binding: STB_GLOBAL
- Name:__ZN3qux3barEii
  Type:STT_FUNC
  Section: .text
  Binding: STB_GLOBAL

Because this expresses the bare minimum of the symbol name, whether its a 
function or an exported object, which section it belongs to, and if it is bound 
globally or locally or weakly.

For the sections, this yaml schema gives us what we need to set the name, type 
and flags properly:

  Sections:
- Name:.text
  Type:SHT_PROGBITS
  Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
- Name:.data
  Type:SHT_PROGBITS
  Flags:   [ SHF_WRITE, SHF_ALLOC ]

We also think that the existing FileHeader works for emitting ELF ifsos:

  
FileHeader:
Class:   ELFCLASS64
Data:ELFDATA2LSB
Type:ET_REL
Machine: EM_X86_64
  


I could see us compacting this (something like "Kind: ELF_64_LSB_X86-64") but I 
don't see why we'd want to deviate from what ELF already lays out.
The file type is important because ET_REL vs ET_DYN can be used to denote if 
this is an ifso artifact that is meant to be merged or if it is already merged. 
I think we could have one stage that does the merging of the ET_RELs and one 
that assembles the final binary from a merged ET_DYN ifso.

We looked at other formats like TBD, and the major thing we also want to add is:

  --- !ifso-elf-v1

at the beginning of the yaml to denote that this is an ifso specifically for 
ELF and to denote the versioning.
I think the versioning could be used to direct how we read the FileHeader.

So the yaml ifso file syntax:

  --- !ifso-elf-
  FileHeader:
Class:   
Data:
Type:
Machine: 
  Sections: 
# One or more of these:
- Name:
  Type:
  Flags:   [  ]
  Symbols:
# One or more of these:
- Name:
  Type:
  Section: 
  Binding: 

How does this sound?

PL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

I think I know the format that I'd like *roughly*. I haven't worked out how to 
handle COMDAT and section groups yet. Roland seems to think that COMDAT and 
section groups will be an issue but I'm working out the details with him to 
make sure I understand it. I'll get back to you soon.

  Version: 1.0
  Arch: AArch64
  Symbols:
- Name: foo
  Type: Function
  Weak: true
- Name: bar
  Type: Function
  Defined: false
- Name: baz
  Type: Object
  Size: 256
  Alignment: 32

Only Arch is used other details from your header can be inferred as defaults 
but are not needed here. I think binding is not needed beyond Weak vs. Not 
Weak. I spoke with Roland about this and he pointed out STB_GNU_UNIQUE. This 
would motivate allowing Global, Weak, and GNU_UNIQUE as valid bindings. The 
only cases for viability that we care are protected and default. Internal and 
hidden will never appear in the public interface. We can treat protected and 
default as the same as they just mean that the symbol will appear in the public 
interface and their difference is related to how the linker resolves symbols 
relative to module itself not other external modules.

We don't have to worry about symbol version (and the TextAPI types don't 
support that yet either) but Warnings are another case to think about. Each 
symbol can have a warning string associated with it. This will be one of the 
last features (before symbol versions but after almost everyhing else) that 
support is added for in llvm-elfabi so I think we can ignore it here for now.

One thing we forgot to add in llvm-elfabi that we now know we need to add is 
Alignment. Alignment can be computed from the section and the offset within the 
section. This is the one bit of information that comes from the section itself 
and not the symbol table.

Absolute symbols probably aren't needed but if we ever need them they're easy 
enough to add via a new symbol type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

In D60974#1483164 , @plotfi wrote:

> So, @compnerd and I have discussed and we'd like to propose a yaml schema 
> that can express what we need for ifsos for the Sections and the Symbols.
>
> ...


Can you clarify is this is for the fully merged format or for the unmerged 
format? I *really* strongly believe that the fully merged format should go 
though llvm-elfabi.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

To clarify my format is for the unmerged format. The merged format should be 
the ELFStub internal representation found in llvm's TextAPI. The plan is to 
have at minimum .tbe and ELF stubs. I'm actively working on getting the ELF 
stub output which can then be converted to the yaml2obj format since they will 
just be ELF files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1483210 , @jakehehrlich 
wrote:

> In D60974#1483164 , @plotfi wrote:
>
> > So, @compnerd and I have discussed and we'd like to propose a yaml schema 
> > that can express what we need for ifsos for the Sections and the Symbols.
> >
> > ...
>
>
> Can you clarify is this is for the fully merged format or for the unmerged 
> format? I *really* strongly believe that the fully merged format should go 
> though llvm-elfabi.


Unmerged. The merged format could be elfabi or raw elf, but I want to agree on 
something that just lets us generate the intermediate unmerged artifacts for 
now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1483209 , @jakehehrlich 
wrote:

>   I think I know the format that I'd like *roughly*. I haven't worked out how 
> to handle COMDAT and section groups yet. Roland seems to think that COMDAT 
> and section groups will be an issue but I'm working out the details with him 
> to make sure I understand it. I'll get back to you soon.
>   
>
>   Version: 1.0
>   Arch: AArch64
>   Symbols:
> - Name: foo
>   Type: Function
>   Weak: true
> - Name: bar
>   Type: Function
>   Defined: false
> - Name: baz
>   Type: Object
>   Size: 256
>   Alignment: 32
>
>
>


Me and @compnerd had discussed a more abstracted format like this but decided 
it was best to just use the same names that are in the ELF already.
Is there any compelling reason not to?
As far as I understand, by having something like "Weak: true" is already 
harking back to ELF so why not stick to the same names?

I think the !tbd-elf-v1 versioning can help with any changes or alterations we 
want to make along the way too.
We did discuss the alignment field too.

> Only Arch is used other details from your header can be inferred as defaults 
> but are not needed here. I think binding is not needed beyond Weak vs. Not 
> Weak. I spoke with Roland about this and he pointed out STB_GNU_UNIQUE. This 
> would motivate allowing Global, Weak, and GNU_UNIQUE as valid bindings. The 
> only cases for viability that we care are protected and default. Internal and 
> hidden will never appear in the public interface. We can treat protected and 
> default as the same as they just mean that the symbol will appear in the 
> public interface and their difference is related to how the linker resolves 
> symbols relative to module itself not other external modules.
> 
> We don't have to worry about symbol version (and the TextAPI types don't 
> support that yet either) but Warnings are another case to think about. Each 
> symbol can have a warning string associated with it. This will be one of the 
> last features (before symbol versions but after almost everyhing else) that 
> support is added for in llvm-elfabi so I think we can ignore it here for now.
> 
> One thing we forgot to add in llvm-elfabi that we now know we need to add is 
> Alignment. Alignment can be computed from the section and the offset within 
> the section. This is the one bit of information that comes from the section 
> itself and not the symbol table.
> 
> Absolute symbols probably aren't needed but if we ever need them they're easy 
> enough to add via a new symbol type.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

In D60974#1483240 , @plotfi wrote:

> Me and @compnerd had discussed a more abstracted format like this but decided 
> it was best to just use the same names that are in the ELF already.
>  Is there any compelling reason not to?
>  As far as I understand, by having something like "Weak: true" is already 
> harking back to ELF so why not stick to the same names?
>
> I think the !tbd-elf-v1 versioning can help with any changes or alterations 
> we want to make along the way too.
>  We did discuss the alignment field too.


The format will have to be ELF specific but that doesn't mean we have to use 
the exact names. The benefit of this format is that you can only do the 
intended thing with it while anything more. This is also the format that 
matches most closely with .tbe which is a plus for consistency of this and 
integration of both tools into the llvm ecosystem. It's obvious how to merge my 
format into the ELFStub format. Your format has extraneous details that would 
never matter to the creation of the ELFStub format like the name of the section 
a symbol was defined in. Also I think much more of the compiler has to be 
considered to get section names right unless you're just recomputing them and 
then that's redundant for no gain.

When we first reviewed the .tbe format I was on the "just use ELF name" side of 
things for the Type and Arch fields. I don't remember why that wasn't chosen 
but for consistency with that choice I think we should do the same thing here. 
As for fields like Weak vs not Weak distilling the format down to only allow 
for those options is a design benefit. not Weak means (GLOBAL or GNU_UNIQUE) 
and Weak means WEAK. Likewise by not messing with visability (you don't mess 
with it either, I'm just pointing this out) we split things into (default | 
protected) and (internal | hidden). These grouped concepts, though exactly what 
we need, are not what available concepts. We also constrain useless bindings 
from existing like LOCAL which isn't meaningful in this context.

Every bit of my format is needed to produce the final stub in working form. All 
section information in your format, some bindings, and ELF file type are all 
not needed. There's an argument for embedding the ELF encoding format in this 
file but that will only be used later once linking occurs. It can also be 
inferred by the architecture in most cases and otherwise specified using a bfd 
output format name when that's what people need/want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

> I think the !tbd-elf-v1 versioning can help with any changes or alterations 
> we want to make along the way too.

I think this is good. I kind of glanced over it and mentioned a Version field. 
Version is handled explicitly by a VersionTuple in llvm-elfabi. I think we 
should do the same thing here. I forget all the details of how this works but 
certainly versioning is a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added inline comments.



Comment at: clang/include/clang/Driver/Types.def:91
 TYPE("ast",  AST,  INVALID, "ast",   "u")
+TYPE("ifso", IFSO, INVALID, "ifso",  "u")
 TYPE("pcm",  ModuleFile,   INVALID, "pcm",   "u")

I think a different name should be used for a few reasons

1) ifso is the name of a tool used internally at Google that isn't anything 
like this. I'd like to avoid confusion or anything related.
2) .tbe is the format name for the textual fully linked format in llvm and I 
feel like we should hearken back to this but this isn't critical really.
3) the end is "so" but this is the intermediate format should it should not 
have the "so".

I think ".tbo" or ".tbe.o" or something like that would be my preferred sort of 
name. This is also in holding with ".tbd" which is the text based version of 
".dnylib" on MacOSX. so it's generally consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1483209 , @jakehehrlich 
wrote:

>   I think I know the format that I'd like *roughly*. I haven't worked out how 
> to handle COMDAT and section groups yet. Roland seems to think that COMDAT 
> and section groups will be an issue but I'm working out the details with him 
> to make sure I understand it. I'll get back to you soon.
>   
>
>   Version: 1.0
>   Arch: AArch64
>   Symbols:
> - Name: foo
>   Type: Function
>   Weak: true
> - Name: bar
>   Type: Function
>   Defined: false
> - Name: baz
>   Type: Object
>   Size: 256
>   Alignment: 32
>
>
>


Also, curious on the lack of denoting the sections and which symbols go in 
which sections? Do you just assume that "Type: Object" goes into .data?

> Only Arch is used other details from your header can be inferred as defaults 
> but are not needed here. I think binding is not needed beyond Weak vs. Not 
> Weak. I spoke with Roland about this and he pointed out STB_GNU_UNIQUE. This 
> would motivate allowing Global, Weak, and GNU_UNIQUE as valid bindings. The 
> only cases for viability that we care are protected and default. Internal and 
> hidden will never appear in the public interface. We can treat protected and 
> default as the same as they just mean that the symbol will appear in the 
> public interface and their difference is related to how the linker resolves 
> symbols relative to module itself not other external modules.
> 
> We don't have to worry about symbol version (and the TextAPI types don't 
> support that yet either) but Warnings are another case to think about. Each 
> symbol can have a warning string associated with it. This will be one of the 
> last features (before symbol versions but after almost everyhing else) that 
> support is added for in llvm-elfabi so I think we can ignore it here for now.
> 
> One thing we forgot to add in llvm-elfabi that we now know we need to add is 
> Alignment. Alignment can be computed from the section and the offset within 
> the section. This is the one bit of information that comes from the section 
> itself and not the symbol table.
> 
> Absolute symbols probably aren't needed but if we ever need them they're easy 
> enough to add via a new symbol type.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D60974#1483265 , @jakehehrlich 
wrote:

> In D60974#1483240 , @plotfi wrote:
>
> > Me and @compnerd had discussed a more abstracted format like this but 
> > decided it was best to just use the same names that are in the ELF already.
> >  Is there any compelling reason not to?
> >  As far as I understand, by having something like "Weak: true" is already 
> > harking back to ELF so why not stick to the same names?
> >
> > I think the !tbd-elf-v1 versioning can help with any changes or alterations 
> > we want to make along the way too.
> >  We did discuss the alignment field too.
>
>
> The format will have to be ELF specific but that doesn't mean we have to use 
> the exact names. The benefit of this format is that you can only do the 
> intended thing with it while anything more. This is also the format that 
> matches most closely with .tbe which is a plus for consistency of this and 
> integration of both tools into the llvm ecosystem. It's obvious how to merge 
> my format into the ELFStub format. Your format has extraneous details that 
> would never matter to the creation of the ELFStub format like the name of the 
> section a symbol was defined in. Also I think much more of the compiler has 
> to be considered to get section names right unless you're just recomputing 
> them and then that's redundant for no gain.


We wanted to use the same names because its just a lot easier understand what 
is if you've already looked at the ELF header code (ie STT_FUNC vs Function).

> When we first reviewed the .tbe format I was on the "just use ELF name" side 
> of things for the Type and Arch fields. I don't remember why that wasn't 
> chosen but for consistency with that choice I think we should do the same 
> thing here. As for fields like Weak vs not Weak distilling the format down to 
> only allow for those options is a design benefit. not Weak means (GLOBAL or 
> GNU_UNIQUE) and Weak means WEAK. Likewise by not messing with visability (you 
> don't mess with it either, I'm just pointing this out) we split things into 
> (default | protected) and (internal | hidden). These grouped concepts, though 
> exactly what we need, are not what available concepts. We also constrain 
> useless bindings from existing like LOCAL which isn't meaningful in this 
> context.
> 
> Every bit of my format is needed to produce the final stub in working form. 
> All section information in your format, some bindings, and ELF file type are 
> all not needed. There's an argument for embedding the ELF encoding format in 
> this file but that will only be used later once linking occurs. It can also 
> be inferred by the architecture in most cases and otherwise specified using a 
> bfd output format name when that's what people need/want.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

In D60974#1483380 , @plotfi wrote:

> Also, curious on the lack of denoting the sections and which symbols go in 
> which sections? Do you just assume that "Type: Object" goes into .data?


You don't need that information and the linker doesn't use it. It only uses 
that information to get alignment. In the code that I'm writing right now it 
happens to put everything in a single rodata section (except for tls which it 
puts in a TLS rodata section, which isn't a thing that even exists in the wild)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

In D60974#1483399 , @plotfi wrote:

> In D60974#1483265 , @jakehehrlich 
> wrote:
>
> > In D60974#1483240 , @plotfi wrote:
> >
> > > Me and @compnerd had discussed a more abstracted format like this but 
> > > decided it was best to just use the same names that are in the ELF 
> > > already.
> > >  Is there any compelling reason not to?
> > >  As far as I understand, by having something like "Weak: true" is already 
> > > harking back to ELF so why not stick to the same names?
> > >
> > > I think the !tbd-elf-v1 versioning can help with any changes or 
> > > alterations we want to make along the way too.
> > >  We did discuss the alignment field too.
> >
> >
> > The format will have to be ELF specific but that doesn't mean we have to 
> > use the exact names. The benefit of this format is that you can only do the 
> > intended thing with it while anything more. This is also the format that 
> > matches most closely with .tbe which is a plus for consistency of this and 
> > integration of both tools into the llvm ecosystem. It's obvious how to 
> > merge my format into the ELFStub format. Your format has extraneous details 
> > that would never matter to the creation of the ELFStub format like the name 
> > of the section a symbol was defined in. Also I think much more of the 
> > compiler has to be considered to get section names right unless you're just 
> > recomputing them and then that's redundant for no gain.
>
>
> We wanted to use the same names because its just a lot easier understand what 
> is if you've already looked at the ELF header code (ie STT_FUNC vs Function).


This is a reasonable opinion and was my opinion as well. But that isn't the way 
review went for .tbe and so now we have a responsibility to be consistent. This 
is bike shed level stuff. I could care less either way except for consistency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

With respect to group handeling. I think we should just check that the details 
of every symbol across all unmerged files that are being merged are consistent 
and then ignore duplicates.

For symbols that are not defined in sections that are a part of a comdat group 
this is just delaying the error to the linker. For symbols defined in a comdat 
group this is the only way we can be consistent with whatever choice the linker 
makes. In practice every comdat group is the same but if it were different the 
linker is allowed to just pick one arbitrarily. The only way to be consistent 
is to ensure that duplicates are identical. This however means we ignore 
duplicate definitions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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