Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-08-30 Thread Philip Reames via cfe-commits
reames added a comment.

This seems to have landed a couple of days ago without problem, but if anyone 
sees any weird effects in shared builds for Linux, this change is probably the 
culprit.  The last time I tried to do something like this, I had to back out my 
change due to linker errors I never had the time to understand on some of the 
build bots.



Comment at: llvm/trunk/include/llvm/Support/Registry.h:132
@@ +131,3 @@
+  namespace llvm { \
+  template<> void REGISTRY_CLASS::add_node(REGISTRY_CLASS::node *N) { \
+if (Tail) \

Minor style comment: Extracting the body of add_node into an add_node_impl 
which is still static and having the macro just pound out a wrapper would be a 
bit cleaner.  


Repository:
  rL LLVM

https://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-27 Thread John Brawn via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL276856: Adjust Registry interface to not require plugins to 
export a registry (authored by john.brawn).

Repository:
  rL LLVM

https://reviews.llvm.org/D21385

Files:
  cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt
  cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt
  cfe/trunk/include/clang/Frontend/FrontendPluginRegistry.h
  cfe/trunk/include/clang/Lex/Preprocessor.h
  cfe/trunk/lib/Frontend/FrontendAction.cpp
  cfe/trunk/lib/Lex/Preprocessor.cpp
  cfe/trunk/lib/Tooling/CompilationDatabase.cpp
  llvm/trunk/include/llvm/Support/Registry.h
  llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
  llvm/trunk/lib/CodeGen/GCStrategy.cpp

Index: llvm/trunk/include/llvm/Support/Registry.h
===
--- llvm/trunk/include/llvm/Support/Registry.h
+++ llvm/trunk/include/llvm/Support/Registry.h
@@ -69,13 +69,14 @@
   node(const entry ) : Next(nullptr), Val(V) {}
 };
 
-static void add_node(node *N) {
-  if (Tail)
-Tail->Next = N;
-  else
-Head = N;
-  Tail = N;
-}
+/// Add a node to the Registry: this is the interface between the plugin and
+/// the executable.
+///
+/// This function is exported by the executable and called by the plugin to
+/// add a node to the executable's registry. Therefore it's not defined here
+/// to avoid it being instantiated in the plugin and is instead defined in
+/// the executable (see LLVM_INSTANTIATE_REGISTRY below).
+static void add_node(node *N);
 
 /// Iterators for registry entries.
 ///
@@ -120,61 +121,23 @@
 add_node();
   }
 };
-
-/// A dynamic import facility.  This is used on Windows to
-/// import the entries added in the plugin.
-static void import(sys::DynamicLibrary , const char *RegistryName) {
-  typedef void *(*GetRegistry)();
-  std::string Name("LLVMGetRegistry_");
-  Name.append(RegistryName);
-  GetRegistry Getter =
-  (GetRegistry)(intptr_t)DL.getAddressOfSymbol(Name.c_str());
-  if (Getter) {
-// Call the getter function in order to get the full copy of the
-// registry defined in the plugin DLL, and copy them over to the
-// current Registry.
-typedef std::pair Info;
-Info *I = static_cast(Getter());
-iterator begin(I->first);
-iterator end(I->second);
-for (++end; begin != end; ++begin) {
-  // This Node object needs to remain alive for the
-  // duration of the program.
-  add_node(new node(*begin));
-}
-  }
-}
-
-/// Retrieve the data to be passed across DLL boundaries when
-/// importing registries from another DLL on Windows.
-static void *exportRegistry() {
-  static std::pair Info(Head, Tail);
-  return 
-}
   };
-
-  
-  // Since these are defined in a header file, plugins must be sure to export
-  // these symbols.
-  template 
-  typename Registry::node *Registry::Head;
-
-  template 
-  typename Registry::node *Registry::Tail;
 } // end namespace llvm
 
-#ifdef LLVM_ON_WIN32
-#define LLVM_EXPORT_REGISTRY(REGISTRY_CLASS)   \
-  extern "C" { \
-  __declspec(dllexport) void *__cdecl LLVMGetRegistry_##REGISTRY_CLASS() { \
-return REGISTRY_CLASS::exportRegistry();   \
-  }\
+/// Instantiate a registry class.
+///
+/// This instantiates add_node and the Head and Tail pointers.
+#define LLVM_INSTANTIATE_REGISTRY(REGISTRY_CLASS) \
+  namespace llvm { \
+  template<> void REGISTRY_CLASS::add_node(REGISTRY_CLASS::node *N) { \
+if (Tail) \
+ Tail->Next = N; \
+else \
+  Head = N; \
+Tail = N; \
+  } \
+  template<> typename REGISTRY_CLASS::node *REGISTRY_CLASS::Head = nullptr; \
+  template<> typename REGISTRY_CLASS::node *REGISTRY_CLASS::Tail = nullptr; \
   }
-#define LLVM_IMPORT_REGISTRY(REGISTRY_CLASS, DL)   \
-  REGISTRY_CLASS::import(DL, #REGISTRY_CLASS)
-#else
-#define LLVM_EXPORT_REGISTRY(REGISTRY_CLASS)
-#define LLVM_IMPORT_REGISTRY(REGISTRY_CLASS, DL)
-#endif
 
 #endif // LLVM_SUPPORT_REGISTRY_H
Index: llvm/trunk/lib/CodeGen/GCStrategy.cpp
===
--- llvm/trunk/lib/CodeGen/GCStrategy.cpp
+++ llvm/trunk/lib/CodeGen/GCStrategy.cpp
@@ -16,6 +16,8 @@
 
 using namespace llvm;
 
+LLVM_INSTANTIATE_REGISTRY(GCRegistry)
+
 GCStrategy::GCStrategy()
 : UseStatepoints(false), NeededSafePoints(0), CustomReadBarriers(false),
   CustomWriteBarriers(false), CustomRoots(false), InitRoots(true),
Index: llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
===
--- 

Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-26 Thread Reid Kleckner via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rL LLVM

https://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-26 Thread Manuel Klimek via cfe-commits
klimek added a reviewer: rnk.
klimek added a comment.

+Reid for a windows reviewer


Repository:
  rL LLVM

https://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-22 Thread John Brawn via cfe-commits
john.brawn added a comment.

Ping.


Repository:
  rL LLVM

https://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-11 Thread John Brawn via cfe-commits
john.brawn added a comment.

> Plugins tests are still disabled by lit.cfg I think, as it only checks for 
> enable_shared, which is disabled. I suppose we could wait for both this and 
> the SampleAnalyzerPlugin to be patched before enabling plugins tests if we 
> have LLVM_EXPORT_SYMBOLS_FOR_PLUGINS, as every plugin test should work after 
> this.


http://reviews.llvm.org/D1 for enabling the tests.

Also: you said LGTM, but didn't mark the review as accepted. OK to commit?


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-08 Thread Rudy Pons via cfe-commits
Ilod added a comment.

LGTM

Plugins tests are still disabled by lit.cfg I think, as it only checks for 
enable_shared, which is disabled. I suppose we could wait for both this and the 
SampleAnalyzerPlugin to be patched before enabling plugins tests if we have 
LLVM_EXPORT_SYMBOLS_FOR_PLUGINS, as every plugin test should work after this.


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-04 Thread John Brawn via cfe-commits
john.brawn updated this revision to Diff 62684.
john.brawn added a comment.

Updated to not touch the sample analyzer plugin.


Repository:
  rL LLVM

http://reviews.llvm.org/D21385

Files:
  cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt
  cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt
  cfe/trunk/include/clang/Frontend/FrontendPluginRegistry.h
  cfe/trunk/include/clang/Lex/Preprocessor.h
  cfe/trunk/lib/Frontend/FrontendAction.cpp
  cfe/trunk/lib/Lex/Preprocessor.cpp
  cfe/trunk/lib/Tooling/CompilationDatabase.cpp
  llvm/trunk/include/llvm/Support/Registry.h
  llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
  llvm/trunk/lib/CodeGen/GCStrategy.cpp

Index: llvm/trunk/lib/CodeGen/GCStrategy.cpp
===
--- llvm/trunk/lib/CodeGen/GCStrategy.cpp
+++ llvm/trunk/lib/CodeGen/GCStrategy.cpp
@@ -16,6 +16,8 @@
 
 using namespace llvm;
 
+LLVM_INSTANTIATE_REGISTRY(GCRegistry)
+
 GCStrategy::GCStrategy()
 : UseStatepoints(false), NeededSafePoints(0), CustomReadBarriers(false),
   CustomWriteBarriers(false), CustomRoots(false), InitRoots(true),
Index: llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
===
--- llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
+++ llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
@@ -14,6 +14,8 @@
 #include "llvm/CodeGen/GCMetadataPrinter.h"
 using namespace llvm;
 
+LLVM_INSTANTIATE_REGISTRY(GCMetadataPrinterRegistry)
+
 GCMetadataPrinter::GCMetadataPrinter() {}
 
 GCMetadataPrinter::~GCMetadataPrinter() {}
Index: llvm/trunk/include/llvm/Support/Registry.h
===
--- llvm/trunk/include/llvm/Support/Registry.h
+++ llvm/trunk/include/llvm/Support/Registry.h
@@ -69,13 +69,14 @@
   node(const entry ) : Next(nullptr), Val(V) {}
 };
 
-static void add_node(node *N) {
-  if (Tail)
-Tail->Next = N;
-  else
-Head = N;
-  Tail = N;
-}
+/// Add a node to the Registry: this is the interface between the plugin and
+/// the executable.
+///
+/// This function is exported by the executable and called by the plugin to
+/// add a node to the executable's registry. Therefore it's not defined here
+/// to avoid it being instantiated in the plugin and is instead defined in
+/// the executable (see LLVM_INSTANTIATE_REGISTRY below).
+static void add_node(node *N);
 
 /// Iterators for registry entries.
 ///
@@ -120,61 +121,23 @@
 add_node();
   }
 };
-
-/// A dynamic import facility.  This is used on Windows to
-/// import the entries added in the plugin.
-static void import(sys::DynamicLibrary , const char *RegistryName) {
-  typedef void *(*GetRegistry)();
-  std::string Name("LLVMGetRegistry_");
-  Name.append(RegistryName);
-  GetRegistry Getter =
-  (GetRegistry)(intptr_t)DL.getAddressOfSymbol(Name.c_str());
-  if (Getter) {
-// Call the getter function in order to get the full copy of the
-// registry defined in the plugin DLL, and copy them over to the
-// current Registry.
-typedef std::pair Info;
-Info *I = static_cast(Getter());
-iterator begin(I->first);
-iterator end(I->second);
-for (++end; begin != end; ++begin) {
-  // This Node object needs to remain alive for the
-  // duration of the program.
-  add_node(new node(*begin));
-}
-  }
-}
-
-/// Retrieve the data to be passed across DLL boundaries when
-/// importing registries from another DLL on Windows.
-static void *exportRegistry() {
-  static std::pair Info(Head, Tail);
-  return 
-}
   };
-
-  
-  // Since these are defined in a header file, plugins must be sure to export
-  // these symbols.
-  template 
-  typename Registry::node *Registry::Head;
-
-  template 
-  typename Registry::node *Registry::Tail;
 } // end namespace llvm
 
-#ifdef LLVM_ON_WIN32
-#define LLVM_EXPORT_REGISTRY(REGISTRY_CLASS)   \
-  extern "C" { \
-  __declspec(dllexport) void *__cdecl LLVMGetRegistry_##REGISTRY_CLASS() { \
-return REGISTRY_CLASS::exportRegistry();   \
-  }\
+/// Instantiate a registry class.
+///
+/// This instantiates add_node and the Head and Tail pointers.
+#define LLVM_INSTANTIATE_REGISTRY(REGISTRY_CLASS) \
+  namespace llvm { \
+  template<> void REGISTRY_CLASS::add_node(REGISTRY_CLASS::node *N) { \
+if (Tail) \
+ Tail->Next = N; \
+else \
+  Head = N; \
+Tail = N; \
+  } \
+  template<> typename REGISTRY_CLASS::node *REGISTRY_CLASS::Head = nullptr; \
+  template<> typename REGISTRY_CLASS::node *REGISTRY_CLASS::Tail = nullptr; \
   }
-#define 

Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-04 Thread John Brawn via cfe-commits
john.brawn added a comment.

Created http://reviews.llvm.org/D21971 to make the sample analyzer plugin work.


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-02 Thread Rudy Pons via cfe-commits
Ilod added a comment.

The PrintFunctionNames plugin works fine, but not the SampleAnalyzerPlugin? The 
checker-plugins test fails. Not sure, but I suppose it's because the 
regitration method for checkers is different (a C method called 
clang_registerCheckers is retrieved in the DLL from the main program, then 
called with a CheckerRegistry - which is not related with the Registry class - 
on which a templated method addChecker is called).

  FAIL: Clang :: Analysis/checker-plugins.c (156 of 9493)
   TEST 'Clang :: Analysis/checker-plugins.c' FAILED 

  Script:
  --
  Debug/bin/clang.EXE -cc1 -internal-isystem 
Debug\bin\..\lib\clang\3.9.0\include -nostdsysteminc -load 
Debug/bin/SampleAnalyzerPlugin.dll -analyze 
-analyzer-checker='example.MainCallChecker' -verify 
tools\clang\test\Analysis\checker-plugins.c
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ "Debug/bin/clang.EXE" "-cc1" "-internal-isystem" 
"Debug\bin\..\lib\clang\3.9.0\include" "-nostdsysteminc" "-load" 
"Debug/bin/SampleAnalyzerPlugin.dll" "-analyze" 
"-analyzer-checker=example.MainCallChecker" "-verify" 
"tools\clang\test\Analysis\checker-plugins.c"
  # command stderr:
  CUSTOMBUILD : error : 'error' diagnostics seen but not expected:
  
(frontend): no analyzer checkers are associated with 
'example.MainCallChecker'
  
  CUSTOMBUILD : error : 'warning' diagnostics expected but not seen:
  
File tools\clang\test\Analysis\checker-plugins.c Line 9: call to main
  
  CUSTOMBUILD : error : 'note' diagnostics seen but not expected:
  
  (frontend): use -analyzer-disable-all-checks to disable all static 
analyzer checkers
  
3 errors generated.
  
  
  CUSTOMBUILD : error : command failed with exit status: 1


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-01 Thread John Brawn via cfe-commits
john.brawn added a comment.

Commit r274365 should make extract_symbols.py work with Python 3.


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-01 Thread John Brawn via cfe-commits
john.brawn added a comment.

> http://clang.llvm.org/get_started.html doesn't specify a version of Python, 
> the LLVM webpage only say >= 2.7, and I ran the tests without any problem 
> with Python 3 (but maybe some tests were automatically skipped?). Also, 
> Python is also marked as only necessary to build the tests, which is no 
> longer true with your script.


Hmm, it looks like this webpage (and 
http://llvm.org/docs/GettingStarted.html#requirements) has been out of date 
since 2011 (commits r143742 and r143793), when python started being necessary 
always as it's used to run llvm-build. Python 3 not being supported appears to 
come from commit r184732 which just says "All of LLVM's Python scripts only 
support Python 2 for widely understood reasons", though I must confess I don't 
know what those reasons are. I'll look into making extract_symbols.py work with 
python 3, though I haven't touched python 3 before now so I don't know how much 
luck I'll have.


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-01 Thread Rudy Pons via cfe-commits
Ilod added a comment.

In http://reviews.llvm.org/D21385#472268, @john.brawn wrote:

> In http://reviews.llvm.org/D21385#471807, @Ilod wrote:
>
> > I gave up after this. I have Pyhton 3.5 64 bits, and was building clang 
> > 32-bits on Windows with Visual Studio 2015 Update 2.
>
>
> python 3 isn't supported when building llvm/clang, according to llvm's 
> CMakeLists.txt.


Oops, will try again tonight with Python 2.
http://clang.llvm.org/get_started.html doesn't specify a version of Python, the 
LLVM webpage only say >= 2.7, and I ran the tests without any problem with 
Python 3 (but maybe some tests were automatically skipped?). Also, Python is 
also marked as only necessary to build the tests, which is no longer true with 
your script.


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-07-01 Thread John Brawn via cfe-commits
john.brawn added a comment.

In http://reviews.llvm.org/D21385#471807, @Ilod wrote:

> I gave up after this. I have Pyhton 3.5 64 bits, and was building clang 
> 32-bits on Windows with Visual Studio 2015 Update 2.


python 3 isn't supported when building llvm/clang, according to llvm's 
CMakeLists.txt.

> If that's all, I suppose it's better to have your limitation to plugin 
> support rather than a full support which will bug if you call a function that 
> accesses a static member.

> 

> (By the way, the previous attempt works fine, it was reverted because of a 
> mingw crash which can be fixed by the patch http://reviews.llvm.org/D18847)

> 

> However, couldn't we enable by default LLVM_EXPORT_SYMBOLS_FOR_PLUGINS on the 
> platforms/compilers which doesn't currently support the plugins? Having 
> plugins only work on custom builds of clang seems to be against the main use 
> of plugins. (If testing is the only problem, I would gladly test it on 
> Windows with mingw, and maybe Mac if needed)


After this patch goes in I plan to email llvm-dev/cfe-dev to see if people 
think that's a good idea.

> Also, could you add in the documentation (cfe/trunk/docs/ClangPlugins.rst) 
> what are the limitations/setup needed to make an out-of-tree plugin?


Will do.


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-06-30 Thread Rudy Pons via cfe-commits
Ilod added a subscriber: Ilod.
Ilod added a comment.

Hello,

When I tried your patch and added LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=ON, I had 
multiple errors on your python scripts, like:

File "utils/extract_symbols.py", line 399, in 
  calling_convention_decoration = is_32bit_windows(libs[0])
File "utils/extract_symbols.py", line 88, in dumpbin_is_32bit_windows
  match = re.match('.+machine \((\S+)\)', line)
File "C:\Program Files\Python35\lib\re.py", line 163, in match
  return _compile(pattern, flags).match(string)
  TypeError: cannot use a string pattern on a bytes-like object

And after I fixed them by passing bytes-like object literals, I had the 
following error:

File "utils/extract_symbols.py", line 488, in 
  print >>outfile, str(k)
  TypeError: unsupported operand type(s) for >>: 'builtin_function_or_method' 
and '_io.TextIOWrapper'

I gave up after this. I have Pyhton 3.5 64 bits, and was building clang 32-bits 
on Windows with Visual Studio 2015 Update 2.

Looking at the patch:

If I understand correctly, the pros/cons between your approach and the previous 
attempt are:

- Your method allows to export the various symbols of the tool from the plugin, 
allowing mainly to access/modify global variables of it, instead of having 
another instance of it in the DLL (thus probably causing many bugs)
- Previous attempt allowed to make a plugin which could be loaded by any tool, 
while yours need the plugin to be specific to a tool

If that's all, I suppose it's better to have your limitation to plugin support 
rather than a full support which will bug if you call a function that accesses 
a static member.

(By the way, the previous attempt works fine, it was reverted because of a 
mingw crash which can be fixed by the patch http://reviews.llvm.org/D18847)

However, couldn't we enable by default LLVM_EXPORT_SYMBOLS_FOR_PLUGINS on the 
platforms/compilers which doesn't currently support the plugins? Having plugins 
only work on custom builds of clang seems to be against the main use of 
plugins. (If testing is the only problem, I would gladly test it on Windows 
with mingw, and maybe Mac if needed)

Also, could you add in the documentation (cfe/trunk/docs/ClangPlugins.rst) what 
are the limitations/setup needed to make an out-of-tree plugin?


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-06-30 Thread John Brawn via cfe-commits
john.brawn added a comment.

Ping


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


Re: [PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-06-23 Thread John Brawn via cfe-commits
john.brawn added a comment.

Ping.


Repository:
  rL LLVM

http://reviews.llvm.org/D21385



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


[PATCH] D21385: Adjust Registry interface to not require plugins to export a registry

2016-06-15 Thread John Brawn via cfe-commits
john.brawn created this revision.
john.brawn added reviewers: ehsan, reames, chapuni.
john.brawn added subscribers: llvm-commits, cfe-commits.
john.brawn set the repository for this revision to rL LLVM.
Herald added a subscriber: klimek.

Currently the Registry class contains the vestiges of a previous attempt to 
allow plugins to be used on Windows without using BUILD_SHARED_LIBS, where a 
plugin would have its own copy of a registry and export it to be imported by 
the tool that's loading the plugin. This only works if the plugin is entirely 
self-contained with the only interface between the plugin and tool being the 
registry, and in particular this conflicts with how IR pass plugins work.

This patch changes things so that instead the add_node function of the registry 
is exported by the tool and then imported by the plugin, which solves this 
problem and also means that instead of every plugin having to export every 
registry they use instead LLVM only has to export the add_node functions. This 
allows plugins that use a registry to work on Windows if 
LLVM_EXPORT_SYMBOLS_FOR_PLUGINS is used.


Repository:
  rL LLVM

http://reviews.llvm.org/D21385

Files:
  cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt
  cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt
  cfe/trunk/examples/analyzer-plugin/CMakeLists.txt
  cfe/trunk/include/clang/Frontend/FrontendPluginRegistry.h
  cfe/trunk/include/clang/Lex/Preprocessor.h
  cfe/trunk/lib/Frontend/FrontendAction.cpp
  cfe/trunk/lib/Lex/Preprocessor.cpp
  cfe/trunk/lib/Tooling/CompilationDatabase.cpp
  llvm/trunk/include/llvm/Support/Registry.h
  llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
  llvm/trunk/lib/CodeGen/GCStrategy.cpp

Index: llvm/trunk/lib/CodeGen/GCStrategy.cpp
===
--- llvm/trunk/lib/CodeGen/GCStrategy.cpp
+++ llvm/trunk/lib/CodeGen/GCStrategy.cpp
@@ -16,6 +16,8 @@
 
 using namespace llvm;
 
+LLVM_INSTANTIATE_REGISTRY(GCRegistry)
+
 GCStrategy::GCStrategy()
 : UseStatepoints(false), NeededSafePoints(0), CustomReadBarriers(false),
   CustomWriteBarriers(false), CustomRoots(false), InitRoots(true),
Index: llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
===
--- llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
+++ llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
@@ -14,6 +14,8 @@
 #include "llvm/CodeGen/GCMetadataPrinter.h"
 using namespace llvm;
 
+LLVM_INSTANTIATE_REGISTRY(GCMetadataPrinterRegistry)
+
 GCMetadataPrinter::GCMetadataPrinter() {}
 
 GCMetadataPrinter::~GCMetadataPrinter() {}
Index: llvm/trunk/include/llvm/Support/Registry.h
===
--- llvm/trunk/include/llvm/Support/Registry.h
+++ llvm/trunk/include/llvm/Support/Registry.h
@@ -69,13 +69,14 @@
   node(const entry ) : Next(nullptr), Val(V) {}
 };
 
-static void add_node(node *N) {
-  if (Tail)
-Tail->Next = N;
-  else
-Head = N;
-  Tail = N;
-}
+/// Add a node to the Registry: this is the interface between the plugin and
+/// the executable.
+///
+/// This function is exported by the executable and called by the plugin to
+/// add a node to the executable's registry. Therefore it's not defined here
+/// to avoid it being instantiated in the plugin and is instead defined in
+/// the executable (see LLVM_INSTANTIATE_REGISTRY below).
+static void add_node(node *N);
 
 /// Iterators for registry entries.
 ///
@@ -120,61 +121,23 @@
 add_node();
   }
 };
-
-/// A dynamic import facility.  This is used on Windows to
-/// import the entries added in the plugin.
-static void import(sys::DynamicLibrary , const char *RegistryName) {
-  typedef void *(*GetRegistry)();
-  std::string Name("LLVMGetRegistry_");
-  Name.append(RegistryName);
-  GetRegistry Getter =
-  (GetRegistry)(intptr_t)DL.getAddressOfSymbol(Name.c_str());
-  if (Getter) {
-// Call the getter function in order to get the full copy of the
-// registry defined in the plugin DLL, and copy them over to the
-// current Registry.
-typedef std::pair Info;
-Info *I = static_cast(Getter());
-iterator begin(I->first);
-iterator end(I->second);
-for (++end; begin != end; ++begin) {
-  // This Node object needs to remain alive for the
-  // duration of the program.
-  add_node(new node(*begin));
-}
-  }
-}
-
-/// Retrieve the data to be passed across DLL boundaries when
-/// importing registries from another DLL on Windows.
-static void *exportRegistry() {
-  static std::pair Info(Head, Tail);
-  return 
-}
   };
-
-  
-  // Since these are defined in a header file, plugins must be sure to export
-  // these symbols.
-  template 
-  typename Registry::node *Registry::Head;
-
-  template 
-