[PATCH] D140504: Link the default GC strategies everywhere getGCStrategy is used

[Campbell Suter via Phabricator via llvm-commits]

znix created this revision.
znix added reviewers: aeubanks, dantrushin, reames.
Herald added a subscriber: hiraditya.
Herald added a project: All.
znix requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

GC strategies are registed using a system of global constructors: any
object can include a GCRegistry::Add stataic variable to register a
strategy, and that will generate a static constructor which registers
this strategy into a global list.

This allows shared libraries to easily register their own strategies,
but poses a problem related to linking: the default GC strategies
(defined and registered in their own file) must obviously be included in
LLVM binaries.

The previous solution was to define an empty functon in BuiltinGCs.cpp,
and call it from LinkAllCodegenComponents.h - this is the solution used
for many other codegen components. This header is then included into the
llc and lli main source files, ensuring everything gets linked into
those binaries.

This isn't great for GCStrategy, which we'd like [1] to use in other
binaries, notably opt for the RS4GC [2] pass. Instead of doing something
specific to opt (for example, adding a call in LinkAllIR), this patch
links to the registry function from getGCStrategy, in the assumption
that anything that might look up a GC strategy probably also expects
the default strategies to exist.

[1] https://reviews.llvm.org/D140458
[2] RewriteStatepointsForGC


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140504

Files:
  llvm/include/llvm/CodeGen/LinkAllCodegenComponents.h
  llvm/lib/IR/GCStrategy.cpp


Index: llvm/lib/IR/GCStrategy.cpp
===================================================================
--- llvm/lib/IR/GCStrategy.cpp
+++ llvm/lib/IR/GCStrategy.cpp
@@ -13,6 +13,7 @@
 
 #include "llvm/IR/GCStrategy.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/IR/BuiltinGCs.h"
 
 using namespace llvm;
 
@@ -25,6 +26,16 @@
     if (S.getName() == Name)
       return S.instantiate();
 
+  // We need to link all the builtin GCs when LLVM is used as a static library.
+  // The linker will quite happily remove the static constructors that register
+  // the builtin GCs if we don't use a function from that object. This function
+  // does nothing but we need to make sure it is (or at least could be, even
+  // with all optimisations enabled) called *somewhere*, and this is a good
+  // place to do that: if the GC strategies are being used then this function
+  // obviously can't be removed by the linker, and here it won't affect
+  // performance, since there's about to be a fatal error anyway.
+  llvm::linkAllBuiltinGCs();
+
   if (GCRegistry::begin() == GCRegistry::end()) {
     // In normal operation, the registry should not be empty.  There should
     // be the builtin GCs if nothing else.  The most likely scenario here is
Index: llvm/include/llvm/CodeGen/LinkAllCodegenComponents.h
===================================================================
--- llvm/include/llvm/CodeGen/LinkAllCodegenComponents.h
+++ llvm/include/llvm/CodeGen/LinkAllCodegenComponents.h
@@ -14,7 +14,6 @@
 #ifndef LLVM_CODEGEN_LINKALLCODEGENCOMPONENTS_H
 #define LLVM_CODEGEN_LINKALLCODEGENCOMPONENTS_H
 
-#include "llvm/IR/BuiltinGCs.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/CodeGen/SchedulerRegistry.h"
 #include "llvm/Target/TargetMachine.h"
@@ -38,8 +37,6 @@
       (void) llvm::createGreedyRegisterAllocator();
       (void) llvm::createDefaultPBQPRegisterAllocator();
 
-      llvm::linkAllBuiltinGCs();
-
       (void) llvm::createBURRListDAGScheduler(nullptr,
                                               llvm::CodeGenOpt::Default);
       (void) llvm::createSourceListDAGScheduler(nullptr,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D140504.484688.patch
Type: text/x-patch
Size: 2111 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221221/f1d37b55/attachment.bin>