Skip to content

[llvm] prepare explicit template instantiations in llvm/CodeGen for DLL export annotations #140653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 20, 2025

Conversation

andrurogerz
Copy link
Contributor

@andrurogerz andrurogerz commented May 20, 2025

Purpose

This patch prepares the llvm/CodeGen library for public interface annotations in support of an LLVM Windows DLL (shared library) build, tracked in #109483. The purpose of this patch is to make the upcoming codemod of this library more straight-forward. It is not expected to impact any functionality.

The LLVM_ABI annotations will be added in a subsequent patch. These changes are required to build with visibility annotations using Clang and gcc on Linux/Darwin/etc; Windows DLL can build fine without them.

Overview

This PR does four things in preparation for adding LLVM_ABI annotations to llvm/CodeGen:

  1. Explicitly include Machine.h and Function.h headers from MachinePassManager.cpp so that Function and Machine types are available for the instantiations of InnerAnalysisManagerProxy. Without this change, Clang only will only export one of the templates after visibility annotations are added to them. Unclear if this is a Clang bug or expected behavior, but this change avoids the issue and should be harmless.
  2. Refactor the definition of MachineFunctionAnalysisManager to its own header file. Without this change, it is not possible to add visibility annotations to the declaration with causing gcc to produce -Wattribute warnings.
  3. Remove the redundant specialization of the DominatorTreeBase<MachineBasicBlock, false>::addRoot method. The specialization is the same as implemented in DominatorTreeBase so should be unnecessary. Without this change, it is not possible to annotate the subsequent instantiations of DominatorTreeBase in the header file without gcc producing -Wattribute warnings. Mark unspecialized addRoot as inline to match the removed specialized version.
  4. Move the explicit instantiations of the GenericDomTreeUpdater template earlier in the header file. These need to appear before being used in the MachineDomTreeUpdater class definition or gcc will produce warnings once visibility annotations are added.

Background

The LLVM Windows DLL effort is tracked in #109483. Additional context is provided in this discourse.

Clang and gcc handle visibility attributes on explicit template instantiations a bit differently; gcc is pickier and generates -Wattribute warnings when an explicit instantiation with a visibility annotation appears after the type has already appeared in the translation unit. These warnings can be avoided by moving explicit template instantiations so they always appear first.

Validation

Local builds and tests to validate cross-platform compatibility. This included llvm, clang, and lldb on the following configurations:

  • Windows with MSVC
  • Windows with Clang
  • Linux with GCC
  • Linux with Clang
  • Darwin with Clang

@andrurogerz andrurogerz marked this pull request as ready for review May 20, 2025 00:47
@andrurogerz
Copy link
Contributor Author

@compnerd, @vgvassilev would one of you mind having a look? This change is intended to be a noop as it is right now, but it will eliminate warnings that appear after export annotations are added.

@llvmbot
Copy link
Member

llvmbot commented May 20, 2025

@llvm/pr-subscribers-llvm-support

Author: Andrew Rogers (andrurogerz)

Changes

Purpose

This patch prepares the llvm/CodeGen library for public interface annotations in support of an LLVM Windows DLL (shared library) build, tracked in #109483. The purpose of this patch is to make the upcoming codemod of this library more straight-forward. It is not expected to impact any functionality.

The LLVM_ABI annotations will be added in a subsequent patch. These changes are required to build with visibility annotations using Clang and gcc on Linux/Darwin/etc; Windows DLL can build fine without them.

Overview

This PR does four things in preparation for adding LLVM_ABI annotations to llvm/CodeGen:

  1. Explicitly include Machine.h and Function.h headers from MachinePassManager.cpp so that Function and Machine types are available for the instantiations of InnerAnalysisManagerProxy. Without this change, Clang only will only export one of the templates after visibility annotations are added to them. Unclear if this is a Clang bug or expected behavior, but this change avoids the issue and should be harmless.
  2. Refactor the definition of MachineFunctionAnalysisManager to its own header file. Without this change, it is not possible to add visibility annotations to the declaration with causing gcc to produce -Wattribute warnings.
  3. Remove the redundant specialization of the DominatorTreeBase&lt;MachineBasicBlock, false&gt;::addRoot method. The specialization is the same as implemented in DominatorTreeBase so should be unnecessary. Without this change, it is not possible to annotate the subsequent instantiations of DominatorTreeBase in the header file without gcc producing -Wattribute warnings.
  4. Move the explicit instantiations of the GenericDomTreeUpdater template earlier in the header file. These need to appear before being used in the MachineDomTreeUpdater class definition or gcc will produce warnings once visibility annotations are added.

Background

The LLVM Windows DLL effort is tracked in #109483. Additional context is provided in this discourse.

Clang and gcc handle visibility attributes on explicit template instantiations a bit differently; gcc is pickier and generates -Wattribute warnings when an explicit instantiation with a visibility annotation appears after the type has already appeared in the translation unit. These warnings can be avoided by moving explicit template instantiations so they always appear first.

Validation

Local builds and tests to validate cross-platform compatibility. This included llvm, clang, and lldb on the following configurations:

  • Windows with MSVC
  • Windows with Clang
  • Linux with GCC
  • Linux with Clang
  • Darwin with Clang

Full diff: https://github.com/llvm/llvm-project/pull/140653.diff

7 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+1-2)
  • (modified) llvm/include/llvm/CodeGen/MachineDomTreeUpdater.h (+16-15)
  • (modified) llvm/include/llvm/CodeGen/MachineDominators.h (-6)
  • (added) llvm/include/llvm/CodeGen/MachineFunctionAnalysisManager.h (+27)
  • (modified) llvm/include/llvm/CodeGen/MachinePassManager.h (+1-4)
  • (modified) llvm/include/llvm/Support/GenericDomTree.h (+1-1)
  • (modified) llvm/lib/CodeGen/MachinePassManager.cpp (+2)
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index 9c563d761c1d9..201e35d30cee2 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -18,6 +18,7 @@
 #include "llvm/ADT/SparseBitVector.h"
 #include "llvm/ADT/ilist.h"
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/CodeGen/MachineFunctionAnalysisManager.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineInstrBundleIterator.h"
 #include "llvm/IR/DebugLoc.h"
@@ -46,8 +47,6 @@ class LiveIntervals;
 class LiveVariables;
 class TargetRegisterClass;
 class TargetRegisterInfo;
-template <typename IRUnitT, typename... ExtraArgTs> class AnalysisManager;
-using MachineFunctionAnalysisManager = AnalysisManager<MachineFunction>;
 
 // This structure uniquely identifies a basic block section.
 // Possible values are
diff --git a/llvm/include/llvm/CodeGen/MachineDomTreeUpdater.h b/llvm/include/llvm/CodeGen/MachineDomTreeUpdater.h
index fcdc0becf31c1..ff9f743bd1276 100644
--- a/llvm/include/llvm/CodeGen/MachineDomTreeUpdater.h
+++ b/llvm/include/llvm/CodeGen/MachineDomTreeUpdater.h
@@ -20,6 +20,22 @@
 namespace llvm {
 
 class MachinePostDominatorTree;
+class MachineDomTreeUpdater;
+
+extern template class GenericDomTreeUpdater<
+    MachineDomTreeUpdater, MachineDominatorTree, MachinePostDominatorTree>;
+
+extern template void
+GenericDomTreeUpdater<MachineDomTreeUpdater, MachineDominatorTree,
+                      MachinePostDominatorTree>::recalculate(MachineFunction
+                                                                 &MF);
+
+extern template void GenericDomTreeUpdater<
+    MachineDomTreeUpdater, MachineDominatorTree,
+    MachinePostDominatorTree>::applyUpdatesImpl</*IsForward=*/true>();
+extern template void GenericDomTreeUpdater<
+    MachineDomTreeUpdater, MachineDominatorTree,
+    MachinePostDominatorTree>::applyUpdatesImpl</*IsForward=*/false>();
 
 class MachineDomTreeUpdater
     : public GenericDomTreeUpdater<MachineDomTreeUpdater, MachineDominatorTree,
@@ -61,20 +77,5 @@ class MachineDomTreeUpdater
   /// Returns true if at least one MachineBasicBlock is deleted.
   bool forceFlushDeletedBB();
 };
-
-extern template class GenericDomTreeUpdater<
-    MachineDomTreeUpdater, MachineDominatorTree, MachinePostDominatorTree>;
-
-extern template void
-GenericDomTreeUpdater<MachineDomTreeUpdater, MachineDominatorTree,
-                      MachinePostDominatorTree>::recalculate(MachineFunction
-                                                                 &MF);
-
-extern template void GenericDomTreeUpdater<
-    MachineDomTreeUpdater, MachineDominatorTree,
-    MachinePostDominatorTree>::applyUpdatesImpl</*IsForward=*/true>();
-extern template void GenericDomTreeUpdater<
-    MachineDomTreeUpdater, MachineDominatorTree,
-    MachinePostDominatorTree>::applyUpdatesImpl</*IsForward=*/false>();
 } // namespace llvm
 #endif // LLVM_CODEGEN_MACHINEDOMTREEUPDATER_H
diff --git a/llvm/include/llvm/CodeGen/MachineDominators.h b/llvm/include/llvm/CodeGen/MachineDominators.h
index 61635ff64502d..d373be6263f64 100644
--- a/llvm/include/llvm/CodeGen/MachineDominators.h
+++ b/llvm/include/llvm/CodeGen/MachineDominators.h
@@ -32,12 +32,6 @@ class MachineFunction;
 class Module;
 class raw_ostream;
 
-template <>
-inline void DominatorTreeBase<MachineBasicBlock, false>::addRoot(
-    MachineBasicBlock *MBB) {
-  this->Roots.push_back(MBB);
-}
-
 extern template class DomTreeNodeBase<MachineBasicBlock>;
 extern template class DominatorTreeBase<MachineBasicBlock, false>; // DomTree
 
diff --git a/llvm/include/llvm/CodeGen/MachineFunctionAnalysisManager.h b/llvm/include/llvm/CodeGen/MachineFunctionAnalysisManager.h
new file mode 100644
index 0000000000000..ccf72d996dc4b
--- /dev/null
+++ b/llvm/include/llvm/CodeGen/MachineFunctionAnalysisManager.h
@@ -0,0 +1,27 @@
+//===- llvm/CodeGen/MachineFunctionAnalysisManager.h ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Typedef for MachineFunctionAnalysisManager as an explicit instantiation of
+// AnalysisManager<MachineFunction>.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_CODEGEN_MACHINEFUNCTIONANALYSISMANAGER
+#define LLVM_CODEGEN_MACHINEFUNCTIONANALYSISMANAGER
+
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+
+class MachineFunction;
+
+extern template class AnalysisManager<MachineFunction>;
+using MachineFunctionAnalysisManager = AnalysisManager<MachineFunction>;
+
+} // namespace llvm
+
+#endif
diff --git a/llvm/include/llvm/CodeGen/MachinePassManager.h b/llvm/include/llvm/CodeGen/MachinePassManager.h
index 69b5f6e92940c..57a6c1a65be46 100644
--- a/llvm/include/llvm/CodeGen/MachinePassManager.h
+++ b/llvm/include/llvm/CodeGen/MachinePassManager.h
@@ -24,6 +24,7 @@
 #include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineFunctionAnalysisManager.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/PassManagerInternal.h"
 #include "llvm/Support/Error.h"
@@ -31,10 +32,6 @@
 namespace llvm {
 class Module;
 class Function;
-class MachineFunction;
-
-extern template class AnalysisManager<MachineFunction>;
-using MachineFunctionAnalysisManager = AnalysisManager<MachineFunction>;
 
 /// An RAII based helper class to modify MachineFunctionProperties when running
 /// pass. Define a MFPropsModifier in PassT::run to set
diff --git a/llvm/include/llvm/Support/GenericDomTree.h b/llvm/include/llvm/Support/GenericDomTree.h
index d3f789dd00269..af542bae9f8c6 100644
--- a/llvm/include/llvm/Support/GenericDomTree.h
+++ b/llvm/include/llvm/Support/GenericDomTree.h
@@ -918,7 +918,7 @@ class DominatorTreeBase {
   }
 
 protected:
-  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
+  inline void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
 
   DomTreeNodeBase<NodeT> *createNode(NodeT *BB,
                                      DomTreeNodeBase<NodeT> *IDom = nullptr) {
diff --git a/llvm/lib/CodeGen/MachinePassManager.cpp b/llvm/lib/CodeGen/MachinePassManager.cpp
index 3eed874903f7c..bbe386507fcd2 100644
--- a/llvm/lib/CodeGen/MachinePassManager.cpp
+++ b/llvm/lib/CodeGen/MachinePassManager.cpp
@@ -14,6 +14,8 @@
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionAnalysis.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/Module.h"
 #include "llvm/IR/PassManagerImpl.h"
 
 using namespace llvm;

@compnerd compnerd merged commit 98595cf into llvm:main May 20, 2025
12 checks passed
@andrurogerz andrurogerz deleted the llvm-codegen-templates branch May 20, 2025 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants