[AMDGPU] Fix unittest linking error with LLVM_LINK_LLVM_DYLIB

[hahnjo]

Follow-up to #88257

[llvmbot]

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-amdgpu

Author: Jonas Hahnfeld (hahnjo)

Changes

Follow-up to #88257

---

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

3 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.h (+3-1)
• (modified) llvm/lib/Target/AMDGPU/SIProgramInfo.h (+2-1)
• (modified) llvm/unittests/MC/AMDGPU/SIProgramInfoMCExprs.cpp (+4-8)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.h b/llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.h
index 26229af638f22..0e3bc63919f06 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.h
@@ -18,6 +18,7 @@
 #include "llvm/BinaryFormat/MsgPackDocument.h"
 #include "llvm/Support/AMDGPUMetadata.h"
 #include "llvm/Support/Alignment.h"
+#include "llvm/Support/Compiler.h"
 
 namespace llvm {
 
@@ -61,7 +62,8 @@ class MetadataStreamer {
                                msgpack::MapDocNode Kern) = 0;
 };
 
-class MetadataStreamerMsgPackV4 : public MetadataStreamer {
+class LLVM_EXTERNAL_VISIBILITY MetadataStreamerMsgPackV4
+    : public MetadataStreamer {
 protected:
   std::unique_ptr<msgpack::Document> HSAMetadataDoc =
       std::make_unique<msgpack::Document>();
diff --git a/llvm/lib/Target/AMDGPU/SIProgramInfo.h b/llvm/lib/Target/AMDGPU/SIProgramInfo.h
index c0a353033c3c5..e66e5a194c8b5 100644
--- a/llvm/lib/Target/AMDGPU/SIProgramInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIProgramInfo.h
@@ -17,6 +17,7 @@
 #define LLVM_LIB_TARGET_AMDGPU_SIPROGRAMINFO_H
 
 #include "llvm/IR/CallingConv.h"
+#include "llvm/Support/Compiler.h"
 #include <cstdint>
 
 namespace llvm {
@@ -27,7 +28,7 @@ class MCExpr;
 class MachineFunction;
 
 /// Track resource usage for kernels / entry functions.
-struct SIProgramInfo {
+struct LLVM_EXTERNAL_VISIBILITY SIProgramInfo {
     // Fields set in PGM_RSRC1 pm4 packet.
     const MCExpr *VGPRBlocks = nullptr;
     const MCExpr *SGPRBlocks = nullptr;
diff --git a/llvm/unittests/MC/AMDGPU/SIProgramInfoMCExprs.cpp b/llvm/unittests/MC/AMDGPU/SIProgramInfoMCExprs.cpp
index f2161f71e6e99..a0e03347e6914 100644
--- a/llvm/unittests/MC/AMDGPU/SIProgramInfoMCExprs.cpp
+++ b/llvm/unittests/MC/AMDGPU/SIProgramInfoMCExprs.cpp
@@ -7,9 +7,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "AMDGPUHSAMetadataStreamer.h"
-#include "AMDGPUTargetMachine.h"
-#include "GCNSubtarget.h"
 #include "SIProgramInfo.h"
+#include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
@@ -25,9 +24,8 @@ using namespace llvm;
 
 class SIProgramInfoMCExprsTest : public testing::Test {
 protected:
-  std::unique_ptr<GCNTargetMachine> TM;
+  std::unique_ptr<LLVMTargetMachine> TM;
   std::unique_ptr<LLVMContext> Ctx;
-  std::unique_ptr<GCNSubtarget> ST;
   std::unique_ptr<MachineModuleInfo> MMI;
   std::unique_ptr<MachineFunction> MF;
   std::unique_ptr<Module> M;
@@ -49,7 +47,7 @@ class SIProgramInfoMCExprsTest : public testing::Test {
     const Target *TheTarget = TargetRegistry::lookupTarget(Triple, Error);
     TargetOptions Options;
 
-    TM.reset(static_cast<GCNTargetMachine *>(TheTarget->createTargetMachine(
+    TM.reset(static_cast<LLVMTargetMachine *>(TheTarget->createTargetMachine(
         Triple, CPU, FS, Options, std::nullopt, std::nullopt)));
 
     Ctx = std::make_unique<LLVMContext>();
@@ -59,9 +57,7 @@ class SIProgramInfoMCExprsTest : public testing::Test {
     auto *F = Function::Create(FType, GlobalValue::ExternalLinkage, "Test", *M);
     MMI = std::make_unique<MachineModuleInfo>(TM.get());
 
-    ST = std::make_unique<GCNSubtarget>(TM->getTargetTriple(),
-                                        TM->getTargetCPU(),
-                                        TM->getTargetFeatureString(), *TM);
+    auto *ST = TM->getSubtargetImpl(*F);
 
     MF = std::make_unique<MachineFunction>(*F, *TM, *ST, 1, *MMI);
     PI.reset(*MF.get());

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 73681b8fee930274e6dc11d4471b44666d6d0dfd 853ec603868523e46d873afbdab5f77501916a87 -- llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.h llvm/lib/Target/AMDGPU/SIProgramInfo.h llvm/unittests/MC/AMDGPU/SIProgramInfoMCExprs.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Target/AMDGPU/SIProgramInfo.h b/llvm/lib/Target/AMDGPU/SIProgramInfo.h
index e66e5a194c..fc45ad5a4a 100644
--- a/llvm/lib/Target/AMDGPU/SIProgramInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIProgramInfo.h
@@ -29,87 +29,87 @@ class MachineFunction;
 
 /// Track resource usage for kernels / entry functions.
 struct LLVM_EXTERNAL_VISIBILITY SIProgramInfo {
-    // Fields set in PGM_RSRC1 pm4 packet.
-    const MCExpr *VGPRBlocks = nullptr;
-    const MCExpr *SGPRBlocks = nullptr;
-    uint32_t Priority = 0;
-    uint32_t FloatMode = 0;
-    uint32_t Priv = 0;
-    uint32_t DX10Clamp = 0;
-    uint32_t DebugMode = 0;
-    uint32_t IEEEMode = 0;
-    uint32_t WgpMode = 0; // GFX10+
-    uint32_t MemOrdered = 0; // GFX10+
-    uint32_t RrWgMode = 0;   // GFX12+
-    const MCExpr *ScratchSize = nullptr;
-
-    // State used to calculate fields set in PGM_RSRC2 pm4 packet.
-    uint32_t LDSBlocks = 0;
-    const MCExpr *ScratchBlocks = nullptr;
-
-    // Fields set in PGM_RSRC2 pm4 packet
-    const MCExpr *ScratchEnable = nullptr;
-    uint32_t UserSGPR = 0;
-    uint32_t TrapHandlerEnable = 0;
-    uint32_t TGIdXEnable = 0;
-    uint32_t TGIdYEnable = 0;
-    uint32_t TGIdZEnable = 0;
-    uint32_t TGSizeEnable = 0;
-    uint32_t TIdIGCompCount = 0;
-    uint32_t EXCPEnMSB = 0;
-    uint32_t LdsSize = 0;
-    uint32_t EXCPEnable = 0;
-
-    const MCExpr *ComputePGMRSrc3GFX90A = nullptr;
-
-    const MCExpr *NumVGPR = nullptr;
-    const MCExpr *NumArchVGPR = nullptr;
-    const MCExpr *NumAccVGPR = nullptr;
-    const MCExpr *AccumOffset = nullptr;
-    uint32_t TgSplit = 0;
-    const MCExpr *NumSGPR = nullptr;
-    unsigned SGPRSpill = 0;
-    unsigned VGPRSpill = 0;
-    uint32_t LDSSize = 0;
-    const MCExpr *FlatUsed = nullptr;
-
-    // Number of SGPRs that meets number of waves per execution unit request.
-    const MCExpr *NumSGPRsForWavesPerEU = nullptr;
-
-    // Number of VGPRs that meets number of waves per execution unit request.
-    const MCExpr *NumVGPRsForWavesPerEU = nullptr;
-
-    // Final occupancy.
-    const MCExpr *Occupancy = nullptr;
-
-    // Whether there is recursion, dynamic allocas, indirect calls or some other
-    // reason there may be statically unknown stack usage.
-    const MCExpr *DynamicCallStack = nullptr;
-
-    // Bonus information for debugging.
-    const MCExpr *VCCUsed = nullptr;
-
-    SIProgramInfo() = default;
-
-    // The constructor sets the values for each member as shown in the struct.
-    // However, setting the MCExpr members to their zero value equivalent
-    // happens in reset together with (duplicated) value re-set for the
-    // non-MCExpr members.
-    void reset(const MachineFunction &MF);
-
-    /// Compute the value of the ComputePGMRsrc1 register.
-    uint64_t getComputePGMRSrc1(const GCNSubtarget &ST) const;
-    uint64_t getPGMRSrc1(CallingConv::ID CC, const GCNSubtarget &ST) const;
-    const MCExpr *getComputePGMRSrc1(const GCNSubtarget &ST,
-                                     MCContext &Ctx) const;
-    const MCExpr *getPGMRSrc1(CallingConv::ID CC, const GCNSubtarget &ST,
-                              MCContext &Ctx) const;
-
-    /// Compute the value of the ComputePGMRsrc2 register.
-    uint64_t getComputePGMRSrc2() const;
-    uint64_t getPGMRSrc2(CallingConv::ID CC) const;
-    const MCExpr *getComputePGMRSrc2(MCContext &Ctx) const;
-    const MCExpr *getPGMRSrc2(CallingConv::ID CC, MCContext &Ctx) const;
+  // Fields set in PGM_RSRC1 pm4 packet.
+  const MCExpr *VGPRBlocks = nullptr;
+  const MCExpr *SGPRBlocks = nullptr;
+  uint32_t Priority = 0;
+  uint32_t FloatMode = 0;
+  uint32_t Priv = 0;
+  uint32_t DX10Clamp = 0;
+  uint32_t DebugMode = 0;
+  uint32_t IEEEMode = 0;
+  uint32_t WgpMode = 0;    // GFX10+
+  uint32_t MemOrdered = 0; // GFX10+
+  uint32_t RrWgMode = 0;   // GFX12+
+  const MCExpr *ScratchSize = nullptr;
+
+  // State used to calculate fields set in PGM_RSRC2 pm4 packet.
+  uint32_t LDSBlocks = 0;
+  const MCExpr *ScratchBlocks = nullptr;
+
+  // Fields set in PGM_RSRC2 pm4 packet
+  const MCExpr *ScratchEnable = nullptr;
+  uint32_t UserSGPR = 0;
+  uint32_t TrapHandlerEnable = 0;
+  uint32_t TGIdXEnable = 0;
+  uint32_t TGIdYEnable = 0;
+  uint32_t TGIdZEnable = 0;
+  uint32_t TGSizeEnable = 0;
+  uint32_t TIdIGCompCount = 0;
+  uint32_t EXCPEnMSB = 0;
+  uint32_t LdsSize = 0;
+  uint32_t EXCPEnable = 0;
+
+  const MCExpr *ComputePGMRSrc3GFX90A = nullptr;
+
+  const MCExpr *NumVGPR = nullptr;
+  const MCExpr *NumArchVGPR = nullptr;
+  const MCExpr *NumAccVGPR = nullptr;
+  const MCExpr *AccumOffset = nullptr;
+  uint32_t TgSplit = 0;
+  const MCExpr *NumSGPR = nullptr;
+  unsigned SGPRSpill = 0;
+  unsigned VGPRSpill = 0;
+  uint32_t LDSSize = 0;
+  const MCExpr *FlatUsed = nullptr;
+
+  // Number of SGPRs that meets number of waves per execution unit request.
+  const MCExpr *NumSGPRsForWavesPerEU = nullptr;
+
+  // Number of VGPRs that meets number of waves per execution unit request.
+  const MCExpr *NumVGPRsForWavesPerEU = nullptr;
+
+  // Final occupancy.
+  const MCExpr *Occupancy = nullptr;
+
+  // Whether there is recursion, dynamic allocas, indirect calls or some other
+  // reason there may be statically unknown stack usage.
+  const MCExpr *DynamicCallStack = nullptr;
+
+  // Bonus information for debugging.
+  const MCExpr *VCCUsed = nullptr;
+
+  SIProgramInfo() = default;
+
+  // The constructor sets the values for each member as shown in the struct.
+  // However, setting the MCExpr members to their zero value equivalent
+  // happens in reset together with (duplicated) value re-set for the
+  // non-MCExpr members.
+  void reset(const MachineFunction &MF);
+
+  /// Compute the value of the ComputePGMRsrc1 register.
+  uint64_t getComputePGMRSrc1(const GCNSubtarget &ST) const;
+  uint64_t getPGMRSrc1(CallingConv::ID CC, const GCNSubtarget &ST) const;
+  const MCExpr *getComputePGMRSrc1(const GCNSubtarget &ST,
+                                   MCContext &Ctx) const;
+  const MCExpr *getPGMRSrc1(CallingConv::ID CC, const GCNSubtarget &ST,
+                            MCContext &Ctx) const;
+
+  /// Compute the value of the ComputePGMRsrc2 register.
+  uint64_t getComputePGMRSrc2() const;
+  uint64_t getPGMRSrc2(CallingConv::ID CC) const;
+  const MCExpr *getComputePGMRSrc2(MCContext &Ctx) const;
+  const MCExpr *getPGMRSrc2(CallingConv::ID CC, MCContext &Ctx) const;
 };
 
 } // namespace llvm

[JanekvO]

Hey, thank you for the patch!
I'm actually not against removing the unittest altogether as described in #88257 (comment). What I'm trying to test should be possible in a couple patches through regular regression tests. Do let me know if there's a preference to keep such a unittest in, though!

[hahnjo]

Hi @JanekvO, fully removing the unittest of course also works. I'm not really familiar with the AMDGPU target nor MC parts of LLVM, just happened to run into linking errors this morning.