[AMDGPU] Remove SIProgramInfo related MCExpr unittest #91758
Conversation
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-mc Author: Janek van Oirschot (JanekvO) ChangesRemoves unittest introduced in #88257. I think it'd be better to wait for the patch that converts AMDGPUResourceUsageAnalysis pass to a function pass and adds the required MC layer infrastructure so testing unresolvable MCExpr can happen as regression tests. Having said that, if the preference is to keep this test in we can move forward with #91727 (+ patches to fix the broken buildbot #88257 (comment)). Full diff: https://github.com/llvm/llvm-project/pull/91758.diff 4 Files Affected:
diff --git a/llvm/unittests/MC/AMDGPU/CMakeLists.txt b/llvm/unittests/MC/AMDGPU/CMakeLists.txt
index be8ff572e6f7d..06ca89a72a7cd 100644
--- a/llvm/unittests/MC/AMDGPU/CMakeLists.txt
+++ b/llvm/unittests/MC/AMDGPU/CMakeLists.txt
@@ -1,20 +1,12 @@
-include_directories(
- ${PROJECT_SOURCE_DIR}/lib/Target/AMDGPU
- ${PROJECT_BINARY_DIR}/lib/Target/AMDGPU
- )
-
set(LLVM_LINK_COMPONENTS
AMDGPUCodeGen
AMDGPUDesc
AMDGPUInfo
- CodeGen
- Core
MC
Support
TargetParser
)
-add_llvm_unittest(AMDGPUMCTests
+add_llvm_unittest(AMDGPUDwarfTests
DwarfRegMappings.cpp
- SIProgramInfoMCExprs.cpp
)
diff --git a/llvm/unittests/MC/AMDGPU/SIProgramInfoMCExprs.cpp b/llvm/unittests/MC/AMDGPU/SIProgramInfoMCExprs.cpp
deleted file mode 100644
index f2161f71e6e99..0000000000000
--- a/llvm/unittests/MC/AMDGPU/SIProgramInfoMCExprs.cpp
+++ /dev/null
@@ -1,81 +0,0 @@
-//===- llvm/unittests/MC/AMDGPU/SIProgramInfoMCExprs.cpp ------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#include "AMDGPUHSAMetadataStreamer.h"
-#include "AMDGPUTargetMachine.h"
-#include "GCNSubtarget.h"
-#include "SIProgramInfo.h"
-#include "llvm/CodeGen/MachineModuleInfo.h"
-#include "llvm/MC/MCContext.h"
-#include "llvm/MC/MCExpr.h"
-#include "llvm/MC/MCStreamer.h"
-#include "llvm/MC/MCSymbol.h"
-#include "llvm/MC/MCTargetOptions.h"
-#include "llvm/MC/TargetRegistry.h"
-#include "llvm/Support/TargetSelect.h"
-#include "llvm/Target/TargetMachine.h"
-#include "gtest/gtest.h"
-
-using namespace llvm;
-
-class SIProgramInfoMCExprsTest : public testing::Test {
-protected:
- std::unique_ptr<GCNTargetMachine> 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;
-
- SIProgramInfo PI;
-
- static void SetUpTestSuite() {
- LLVMInitializeAMDGPUTargetInfo();
- LLVMInitializeAMDGPUTarget();
- LLVMInitializeAMDGPUTargetMC();
- }
-
- SIProgramInfoMCExprsTest() {
- std::string Triple = "amdgcn-amd-amdhsa";
- std::string CPU = "gfx1010";
- std::string FS = "";
-
- std::string Error;
- const Target *TheTarget = TargetRegistry::lookupTarget(Triple, Error);
- TargetOptions Options;
-
- TM.reset(static_cast<GCNTargetMachine *>(TheTarget->createTargetMachine(
- Triple, CPU, FS, Options, std::nullopt, std::nullopt)));
-
- Ctx = std::make_unique<LLVMContext>();
- M = std::make_unique<Module>("Module", *Ctx);
- M->setDataLayout(TM->createDataLayout());
- auto *FType = FunctionType::get(Type::getVoidTy(*Ctx), false);
- 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);
-
- MF = std::make_unique<MachineFunction>(*F, *TM, *ST, 1, *MMI);
- PI.reset(*MF.get());
- }
-};
-
-TEST_F(SIProgramInfoMCExprsTest, TestDeathHSAKernelEmit) {
- MCContext &Ctx = MF->getContext();
- MCSymbol *Sym = Ctx.getOrCreateSymbol("Unknown");
- PI.ScratchSize = MCSymbolRefExpr::create(Sym, Ctx);
-
- auto &Func = MF->getFunction();
- Func.setCallingConv(CallingConv::AMDGPU_KERNEL);
- AMDGPU::HSAMD::MetadataStreamerMsgPackV4 MD;
- EXPECT_DEATH(MD.emitKernel(*MF, PI),
- "could not resolve expression when required.");
-}
diff --git a/llvm/utils/gn/secondary/llvm/lib/Target/AMDGPU/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/Target/AMDGPU/BUILD.gn
index dad4f028236d8..edd8d4f1840d0 100644
--- a/llvm/utils/gn/secondary/llvm/lib/Target/AMDGPU/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/lib/Target/AMDGPU/BUILD.gn
@@ -60,7 +60,6 @@ tablegen("AMDGPUGenMCPseudoLowering") {
tablegen("AMDGPUGenRegisterBank") {
visibility = [
":LLVMAMDGPUCodeGen",
- "MCTargetDesc",
"Utils",
"//llvm/unittests/MC/AMDGPU:AMDGPUMCTests",
"//llvm/unittests/Target/AMDGPU:AMDGPUTests",
diff --git a/llvm/utils/gn/secondary/llvm/lib/Target/AMDGPU/MCTargetDesc/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/Target/AMDGPU/MCTargetDesc/BUILD.gn
index 0df55cbc08262..5ba91fcec83a0 100644
--- a/llvm/utils/gn/secondary/llvm/lib/Target/AMDGPU/MCTargetDesc/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/lib/Target/AMDGPU/MCTargetDesc/BUILD.gn
@@ -94,10 +94,6 @@ static_library("MCTargetDesc") {
"//llvm/lib/Target/AMDGPU/TargetInfo",
"//llvm/lib/Target/AMDGPU/Utils",
"//llvm/lib/TargetParser",
-
- # AMDGPUMCExpr.cpp includes GCNSubtarget.h which after 490e348e679
- # includes the generated AMDGPUGenRegisterBank.inc file :/
- "//llvm/lib/Target/AMDGPU/:AMDGPUGenRegisterBank",
]
include_dirs = [ ".." ]
sources = [
|
|
We already have the build fix, why drop test coverage |
When I was adding this unittest at the time I was already feeling a bit conflicted about this unittest, so I may be biased. I believe that eventually this unittest will either have to be changed/removed regardless. The unittest currently serves the purpose of testing the conversion of MCExprs back to known values through the MCExpr's Again, if the desire to keep this unittest in then that's fine as well and we can move forward with #91727. |
|
Eventually needing to change means we still need test coverage today. Just leave it |
Removes unittest introduced in #88257. I think it'd be better to wait for the patch that converts AMDGPUResourceUsageAnalysis pass to a function pass and adds the required MC layer infrastructure so testing unresolvable MCExpr can happen as regression tests.
Having said that, if the preference is to keep this test in we can move forward with #91727 (+ patches to fix the broken buildbot #88257 (comment)).