Skip to content

Conversation

@kyulee-com
Copy link
Contributor

While working on a MIR unittest, I noticed that parseMIR includes an unused argument that sets a function name. This is not only redundant but also irrelevant, as parseMIR is designed to parse entire module, not specific functions, even though most unittests contain a single function per module. To streamline the API, I have removed this unnecessary argument from parseMIR. However, if this argument was originally included to enhance readability or for any other purpose, please let me know.

While working on a MIR unittest, I noticed that parseMIR includes an unused argument that sets a function name. This is not only redundant but also irrelevant, as parseMIR is designed to parse entire module, not specific functions, even though most unittests contain a single function per module. To streamline the API, I have removed this unnecessary argument from parseMIR. However, if this argument was originally included to enhance readability or for any other purpose, please let me know.
@llvmbot
Copy link
Member

llvmbot commented Aug 26, 2024

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-webassembly

Author: Kyungwoo Lee (kyulee-com)

Changes

While working on a MIR unittest, I noticed that parseMIR includes an unused argument that sets a function name. This is not only redundant but also irrelevant, as parseMIR is designed to parse entire module, not specific functions, even though most unittests contain a single function per module. To streamline the API, I have removed this unnecessary argument from parseMIR. However, if this argument was originally included to enhance readability or for any other purpose, please let me know.


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

5 Files Affected:

  • (modified) llvm/unittests/CodeGen/GlobalISel/GISelMITest.h (+4-7)
  • (modified) llvm/unittests/CodeGen/MachineDomTreeUpdaterTest.cpp (+3-3)
  • (modified) llvm/unittests/MI/LiveIntervalTest.cpp (+5-3)
  • (modified) llvm/unittests/MIR/MachineMetadata.cpp (+7-7)
  • (modified) llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp (+3-5)
diff --git a/llvm/unittests/CodeGen/GlobalISel/GISelMITest.h b/llvm/unittests/CodeGen/GlobalISel/GISelMITest.h
index fd31e95cce13d9..544dcabb56d0a4 100644
--- a/llvm/unittests/CodeGen/GlobalISel/GISelMITest.h
+++ b/llvm/unittests/CodeGen/GlobalISel/GISelMITest.h
@@ -53,11 +53,9 @@ std::ostream &
 operator<<(std::ostream &OS, const MachineFunction &MF);
 }
 
-static std::unique_ptr<Module> parseMIR(LLVMContext &Context,
-                                        std::unique_ptr<MIRParser> &MIR,
-                                        const TargetMachine &TM,
-                                        StringRef MIRCode, const char *FuncName,
-                                        MachineModuleInfo &MMI) {
+static std::unique_ptr<Module>
+parseMIR(LLVMContext &Context, std::unique_ptr<MIRParser> &MIR,
+         const TargetMachine &TM, StringRef MIRCode, MachineModuleInfo &MMI) {
   SMDiagnostic Diagnostic;
   std::unique_ptr<MemoryBuffer> MBuffer = MemoryBuffer::getMemBuffer(MIRCode);
   MIR = createMIRParser(std::move(MBuffer), Context);
@@ -80,8 +78,7 @@ createDummyModule(LLVMContext &Context, const LLVMTargetMachine &TM,
                   StringRef MIRString, const char *FuncName) {
   std::unique_ptr<MIRParser> MIR;
   auto MMI = std::make_unique<MachineModuleInfo>(&TM);
-  std::unique_ptr<Module> M =
-      parseMIR(Context, MIR, TM, MIRString, FuncName, *MMI);
+  std::unique_ptr<Module> M = parseMIR(Context, MIR, TM, MIRString, *MMI);
   return make_pair(std::move(M), std::move(MMI));
 }
 
diff --git a/llvm/unittests/CodeGen/MachineDomTreeUpdaterTest.cpp b/llvm/unittests/CodeGen/MachineDomTreeUpdaterTest.cpp
index 9dcf3754a5bd74..f8505817d2e09e 100644
--- a/llvm/unittests/CodeGen/MachineDomTreeUpdaterTest.cpp
+++ b/llvm/unittests/CodeGen/MachineDomTreeUpdaterTest.cpp
@@ -73,7 +73,7 @@ class MachineDomTreeUpdaterTest : public testing::Test {
     MAM.registerPass([&] { return MachineModuleAnalysis(*MMI); });
   }
 
-  bool parseMIR(StringRef MIRCode, const char *FnName) {
+  bool parseMIR(StringRef MIRCode) {
     SMDiagnostic Diagnostic;
     std::unique_ptr<MemoryBuffer> MBuffer = MemoryBuffer::getMemBuffer(MIRCode);
     MIR = createMIRParser(std::move(MBuffer), Context);
@@ -149,7 +149,7 @@ body:             |
 ...
 )";
 
-  ASSERT_TRUE(parseMIR(MIRString, "f0"));
+  ASSERT_TRUE(parseMIR(MIRString));
 
   auto &MF =
       FAM.getResult<MachineFunctionAnalysis>(*M->getFunction("f0")).getMF();
@@ -239,7 +239,7 @@ body:             |
 ...
 )";
 
-  ASSERT_TRUE(parseMIR(MIRString, "f0"));
+  ASSERT_TRUE(parseMIR(MIRString));
 
   auto &MF =
       FAM.getResult<MachineFunctionAnalysis>(*M->getFunction("f0")).getMF();
diff --git a/llvm/unittests/MI/LiveIntervalTest.cpp b/llvm/unittests/MI/LiveIntervalTest.cpp
index bba5cb84d11520..7dcd82f3e7aa61 100644
--- a/llvm/unittests/MI/LiveIntervalTest.cpp
+++ b/llvm/unittests/MI/LiveIntervalTest.cpp
@@ -55,8 +55,10 @@ std::unique_ptr<LLVMTargetMachine> createTargetMachine() {
 }
 
 std::unique_ptr<Module> parseMIR(LLVMContext &Context,
-    legacy::PassManagerBase &PM, std::unique_ptr<MIRParser> &MIR,
-    const LLVMTargetMachine &TM, StringRef MIRCode, const char *FuncName) {
+                                 legacy::PassManagerBase &PM,
+                                 std::unique_ptr<MIRParser> &MIR,
+                                 const LLVMTargetMachine &TM,
+                                 StringRef MIRCode) {
   SMDiagnostic Diagnostic;
   std::unique_ptr<MemoryBuffer> MBuffer = MemoryBuffer::getMemBuffer(MIRCode);
   MIR = createMIRParser(std::move(MBuffer), Context);
@@ -209,7 +211,7 @@ static void doTest(StringRef MIRFunc,
 
   legacy::PassManager PM;
   std::unique_ptr<MIRParser> MIR;
-  std::unique_ptr<Module> M = parseMIR(Context, PM, MIR, *TM, MIRFunc, "func");
+  std::unique_ptr<Module> M = parseMIR(Context, PM, MIR, *TM, MIRFunc);
   ASSERT_TRUE(M);
 
   PM.add(new TestPassT<AnalysisType>(T, ShouldPass));
diff --git a/llvm/unittests/MIR/MachineMetadata.cpp b/llvm/unittests/MIR/MachineMetadata.cpp
index 9b1c3ef1c465ad..cd30768f7ce766 100644
--- a/llvm/unittests/MIR/MachineMetadata.cpp
+++ b/llvm/unittests/MIR/MachineMetadata.cpp
@@ -79,7 +79,7 @@ class MachineMetadataTest : public testing::Test {
   }
 
   std::unique_ptr<Module> parseMIR(const TargetMachine &TM, StringRef MIRCode,
-                                   const char *FnName, MachineModuleInfo &MMI) {
+                                   MachineModuleInfo &MMI) {
     SMDiagnostic Diagnostic;
     std::unique_ptr<MemoryBuffer> MBuffer = MemoryBuffer::getMemBuffer(MIRCode);
     MIR = createMIRParser(std::move(MBuffer), Context);
@@ -227,7 +227,7 @@ body:             |
 )MIR";
 
   MachineModuleInfo MMI(TM.get());
-  M = parseMIR(*TM, MIRString, "test0", MMI);
+  M = parseMIR(*TM, MIRString, MMI);
   ASSERT_TRUE(M);
 
   auto *MF = MMI.getMachineFunction(*M->getFunction("test0"));
@@ -338,7 +338,7 @@ body:             |
 )MIR";
 
   MachineModuleInfo MMI(TM.get());
-  M = parseMIR(*TM, MIRString, "test0", MMI);
+  M = parseMIR(*TM, MIRString, MMI);
   ASSERT_TRUE(M);
 
   auto *MF = MMI.getMachineFunction(*M->getFunction("test0"));
@@ -376,7 +376,7 @@ body:             |
 )MIR";
 
   MachineModuleInfo MMI(TM.get());
-  M = parseMIR(*TM, MIRString, "test0", MMI);
+  M = parseMIR(*TM, MIRString, MMI);
   ASSERT_TRUE(M);
 
   auto *MF = MMI.getMachineFunction(*M->getFunction("test0"));
@@ -474,7 +474,7 @@ body:             |
 )MIR";
 
   MachineModuleInfo MMI(TM.get());
-  M = parseMIR(*TM, MIRString, "test0", MMI);
+  M = parseMIR(*TM, MIRString, MMI);
   ASSERT_TRUE(M);
 
   auto *MF = MMI.getMachineFunction(*M->getFunction("test0"));
@@ -563,7 +563,7 @@ body:             |
 ...
 )MIR";
   MachineModuleInfo MMI(TM.get());
-  M = parseMIR(*TM, MIRString, "foo", MMI);
+  M = parseMIR(*TM, MIRString, MMI);
   ASSERT_TRUE(M);
   auto *MF = MMI.getMachineFunction(*M->getFunction("foo"));
   MachineFunctionProperties &Properties = MF->getProperties();
@@ -594,7 +594,7 @@ body:             |
 ...
 )MIR";
   MachineModuleInfo MMI(TM.get());
-  M = parseMIR(*TM, MIRString, "foo", MMI);
+  M = parseMIR(*TM, MIRString, MMI);
   ASSERT_TRUE(M);
   auto *MF = MMI.getMachineFunction(*M->getFunction("foo"));
   MachineFunctionProperties &Properties = MF->getProperties();
diff --git a/llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp b/llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp
index 5838ab6f782ba8..55caaf5d13b6c8 100644
--- a/llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp
+++ b/llvm/unittests/Target/WebAssembly/WebAssemblyExceptionInfoTest.cpp
@@ -43,7 +43,7 @@ std::unique_ptr<LLVMTargetMachine> createTargetMachine() {
 std::unique_ptr<Module> parseMIR(LLVMContext &Context,
                                  std::unique_ptr<MIRParser> &MIR,
                                  const TargetMachine &TM, StringRef MIRCode,
-                                 const char *FuncName, MachineModuleInfo &MMI) {
+                                 MachineModuleInfo &MMI) {
   SMDiagnostic Diagnostic;
   std::unique_ptr<MemoryBuffer> MBuffer = MemoryBuffer::getMemBuffer(MIRCode);
   MIR = createMIRParser(std::move(MBuffer), Context);
@@ -157,8 +157,7 @@ body: |
   LLVMContext Context;
   std::unique_ptr<MIRParser> MIR;
   MachineModuleInfo MMI(TM.get());
-  std::unique_ptr<Module> M =
-      parseMIR(Context, MIR, *TM, MIRString, "test0", MMI);
+  std::unique_ptr<Module> M = parseMIR(Context, MIR, *TM, MIRString, MMI);
   ASSERT_TRUE(M);
 
   Function *F = M->getFunction("test0");
@@ -332,8 +331,7 @@ body: |
   LLVMContext Context;
   std::unique_ptr<MIRParser> MIR;
   MachineModuleInfo MMI(TM.get());
-  std::unique_ptr<Module> M =
-      parseMIR(Context, MIR, *TM, MIRString, "test1", MMI);
+  std::unique_ptr<Module> M = parseMIR(Context, MIR, *TM, MIRString, MMI);
   ASSERT_TRUE(M);
 
   Function *F = M->getFunction("test1");

Copy link
Contributor

@KanRobert KanRobert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kyulee-com kyulee-com merged commit 38c3855 into llvm:main Aug 27, 2024
@kyulee-com kyulee-com deleted the parseMIR branch August 27, 2024 02:19
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