Skip to content
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

[CodeGen] Port DeadMachineInstructionElim to new pass manager #80582

Merged

Conversation

paperchalice
Copy link
Contributor

@paperchalice paperchalice commented Feb 4, 2024

A simple enough op pass so we can test standard instrumentations in future.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 4, 2024

@llvm/pr-subscribers-backend-aarch64

Author: None (paperchalice)

Changes

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

5 Files Affected:

  • (added) llvm/include/llvm/CodeGen/DeadMachineInstructionElim.h (+25)
  • (modified) llvm/include/llvm/Passes/MachinePassRegistry.def (+1-2)
  • (modified) llvm/lib/CodeGen/DeadMachineInstructionElim.cpp (+42-24)
  • (modified) llvm/lib/Passes/PassBuilder.cpp (+1)
  • (modified) llvm/test/CodeGen/AArch64/elim-dead-mi.mir (+1)
diff --git a/llvm/include/llvm/CodeGen/DeadMachineInstructionElim.h b/llvm/include/llvm/CodeGen/DeadMachineInstructionElim.h
new file mode 100644
index 0000000000000..147523c71645f
--- /dev/null
+++ b/llvm/include/llvm/CodeGen/DeadMachineInstructionElim.h
@@ -0,0 +1,25 @@
+//===- llvm/CodeGen/DeadMachineInstructionElim.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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CODEGEN_DEADMACHINEINSTRUCTIONELIM_H
+#define LLVM_CODEGEN_DEADMACHINEINSTRUCTIONELIM_H
+
+#include "llvm/CodeGen/MachinePassManager.h"
+
+namespace llvm {
+
+class DeadMachineInstructionElimPass
+    : MachinePassInfoMixin<DeadMachineInstructionElimPass> {
+public:
+  PreservedAnalyses run(MachineFunction &MF,
+                        MachineFunctionAnalysisManager &MFAM);
+};
+
+} // namespace llvm
+
+#endif // LLVM_CODEGEN_DEADMACHINEINSTRUCTIONELIM_H
diff --git a/llvm/include/llvm/Passes/MachinePassRegistry.def b/llvm/include/llvm/Passes/MachinePassRegistry.def
index e311682a56192..a0da6cb062233 100644
--- a/llvm/include/llvm/Passes/MachinePassRegistry.def
+++ b/llvm/include/llvm/Passes/MachinePassRegistry.def
@@ -125,6 +125,7 @@ MACHINE_FUNCTION_ANALYSIS("pass-instrumentation", PassInstrumentationAnalysis,
 #ifndef MACHINE_FUNCTION_PASS
 #define MACHINE_FUNCTION_PASS(NAME, PASS_NAME, CONSTRUCTOR)
 #endif
+MACHINE_FUNCTION_PASS("dead-mi-elimination", DeadMachineInstructionElimPass, ())
 // MACHINE_FUNCTION_PASS("free-machine-function", FreeMachineFunctionPass, ())
 MACHINE_FUNCTION_PASS("no-op-machine-function", NoOpMachineFunctionPass, ())
 MACHINE_FUNCTION_PASS("print", PrintMIRPass, ())
@@ -165,8 +166,6 @@ DUMMY_MACHINE_FUNCTION_PASS("break-false-deps", BreakFalseDepsPass, ())
 DUMMY_MACHINE_FUNCTION_PASS("cfguard-longjmp", CFGuardLongjmpPass, ())
 DUMMY_MACHINE_FUNCTION_PASS("cfi-fixup", CFIFixupPass, ())
 DUMMY_MACHINE_FUNCTION_PASS("cfi-instr-inserter", CFIInstrInserterPass, ())
-DUMMY_MACHINE_FUNCTION_PASS("dead-mi-elimination",
-                            DeadMachineInstructionElimPass, ())
 DUMMY_MACHINE_FUNCTION_PASS("detect-dead-lanes", DetectDeadLanesPass, ())
 DUMMY_MACHINE_FUNCTION_PASS("dot-machine-cfg", MachineCFGPrinter, ())
 DUMMY_MACHINE_FUNCTION_PASS("early-ifcvt", EarlyIfConverterPass, ())
diff --git a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
index 6a7de3b241fee..facc01452d2f1 100644
--- a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
+++ b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/CodeGen/DeadMachineInstructionElim.h"
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/LiveRegUnits.h"
@@ -28,37 +29,57 @@ using namespace llvm;
 STATISTIC(NumDeletes,          "Number of dead instructions deleted");
 
 namespace {
-  class DeadMachineInstructionElim : public MachineFunctionPass {
-    bool runOnMachineFunction(MachineFunction &MF) override;
+class DeadMachineInstructionElimImpl {
+  const MachineRegisterInfo *MRI = nullptr;
+  const TargetInstrInfo *TII = nullptr;
+  LiveRegUnits LivePhysRegs;
 
-    const MachineRegisterInfo *MRI = nullptr;
-    const TargetInstrInfo *TII = nullptr;
-    LiveRegUnits LivePhysRegs;
+public:
+  bool runImpl(MachineFunction &MF);
 
-  public:
-    static char ID; // Pass identification, replacement for typeid
-    DeadMachineInstructionElim() : MachineFunctionPass(ID) {
-     initializeDeadMachineInstructionElimPass(*PassRegistry::getPassRegistry());
-    }
+private:
+  bool isDead(const MachineInstr *MI) const;
+  bool eliminateDeadMI(MachineFunction &MF);
+};
 
-    void getAnalysisUsage(AnalysisUsage &AU) const override {
-      AU.setPreservesCFG();
-      MachineFunctionPass::getAnalysisUsage(AU);
-    }
+class DeadMachineInstructionElim : public MachineFunctionPass {
+public:
+  static char ID; // Pass identification, replacement for typeid
 
-  private:
-    bool isDead(const MachineInstr *MI) const;
+  DeadMachineInstructionElim() : MachineFunctionPass(ID) {
+    initializeDeadMachineInstructionElimPass(*PassRegistry::getPassRegistry());
+  }
+
+  bool runOnMachineFunction(MachineFunction &MF) override {
+    if (skipFunction(MF.getFunction()))
+      return false;
+    return DeadMachineInstructionElimImpl().runImpl(MF);
+  }
 
-    bool eliminateDeadMI(MachineFunction &MF);
-  };
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesCFG();
+    MachineFunctionPass::getAnalysisUsage(AU);
+  }
+};
+} // namespace
+
+PreservedAnalyses
+DeadMachineInstructionElimPass::run(MachineFunction &MF,
+                                    MachineFunctionAnalysisManager &) {
+  if (!DeadMachineInstructionElimImpl().runImpl(MF))
+    return PreservedAnalyses::all();
+  PreservedAnalyses PA;
+  PA.preserveSet<CFGAnalyses>();
+  return PA;
 }
+
 char DeadMachineInstructionElim::ID = 0;
 char &llvm::DeadMachineInstructionElimID = DeadMachineInstructionElim::ID;
 
 INITIALIZE_PASS(DeadMachineInstructionElim, DEBUG_TYPE,
                 "Remove dead machine instructions", false, false)
 
-bool DeadMachineInstructionElim::isDead(const MachineInstr *MI) const {
+bool DeadMachineInstructionElimImpl::isDead(const MachineInstr *MI) const {
   // Technically speaking inline asm without side effects and no defs can still
   // be deleted. But there is so much bad inline asm code out there, we should
   // let them be.
@@ -102,10 +123,7 @@ bool DeadMachineInstructionElim::isDead(const MachineInstr *MI) const {
   return true;
 }
 
-bool DeadMachineInstructionElim::runOnMachineFunction(MachineFunction &MF) {
-  if (skipFunction(MF.getFunction()))
-    return false;
-
+bool DeadMachineInstructionElimImpl::runImpl(MachineFunction &MF) {
   MRI = &MF.getRegInfo();
 
   const TargetSubtargetInfo &ST = MF.getSubtarget();
@@ -118,7 +136,7 @@ bool DeadMachineInstructionElim::runOnMachineFunction(MachineFunction &MF) {
   return AnyChanges;
 }
 
-bool DeadMachineInstructionElim::eliminateDeadMI(MachineFunction &MF) {
+bool DeadMachineInstructionElimImpl::eliminateDeadMI(MachineFunction &MF) {
   bool AnyChanges = false;
 
   // Loop over all instructions in all blocks, from bottom to top, so that it's
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 0f33af22dbd97..e9a6b8bc7a101 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -76,6 +76,7 @@
 #include "llvm/CodeGen/BasicBlockSectionsProfileReader.h"
 #include "llvm/CodeGen/CallBrPrepare.h"
 #include "llvm/CodeGen/CodeGenPrepare.h"
+#include "llvm/CodeGen/DeadMachineInstructionElim.h"
 #include "llvm/CodeGen/DwarfEHPrepare.h"
 #include "llvm/CodeGen/ExpandLargeDivRem.h"
 #include "llvm/CodeGen/ExpandLargeFpConvert.h"
diff --git a/llvm/test/CodeGen/AArch64/elim-dead-mi.mir b/llvm/test/CodeGen/AArch64/elim-dead-mi.mir
index 0510a8798a49d..dc0d20f3e8ae0 100644
--- a/llvm/test/CodeGen/AArch64/elim-dead-mi.mir
+++ b/llvm/test/CodeGen/AArch64/elim-dead-mi.mir
@@ -1,5 +1,6 @@
 # RUN: llc -mtriple=aarch64 -o - %s \
 # RUN: -run-pass dead-mi-elimination | FileCheck %s
+# RUN: llc -mtriple=aarch64 -o - %s -p dead-mi-elimination | FileCheck %s
 --- |
   @c = internal unnamed_addr global [3 x i8] zeroinitializer, align 4
   @d = common dso_local local_unnamed_addr global i32 0, align 4

@paperchalice paperchalice force-pushed the NPM/CodeGen/dead-mi-elimination branch 2 times, most recently from 3e9824c to 40a715c Compare February 4, 2024 04:22
if (!DeadMachineInstructionElimImpl().runImpl(MF))
return PreservedAnalyses::all();
PreservedAnalyses PA;
PA.preserveSet<CFGAnalyses>();
Copy link
Contributor

Choose a reason for hiding this comment

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

do we also need to mark all the IR passes as preserved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIUC, inner IR pass must use registerOuterAnalysisInvalidation to invalidate analysis result in outer IRUnit, and currently there is no inner/outer analysis proxy in codebase.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm working on this sort of thing in aeubanks@91e4939. We can port passes in the meantime though.

Question: can an MIR function pass ever modify the corresponding IR function? Module MIR passes can introduce dummy IR functions corresponding to new (e.g. outlined) MIR functions, but that's different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently the content of MIR is created by instruction selector from IR, so MIR shouldn't change IR, otherwise it will invalidate the result of instruction selector.
Also

//===----------------------------------------------------------------------===//
//
// This file defines the MachineFunctionPass class. MachineFunctionPass's are
// just FunctionPass's, except they operate on machine code as part of a code
// generator. Because they operate on machine code, not the LLVM
// representation, MachineFunctionPass's are not allowed to modify the LLVM
// representation. Due to this limitation, the MachineFunctionPass class takes
// care of declaring that no LLVM passes are invalidated.
//
//===----------------------------------------------------------------------===//

if (!DeadMachineInstructionElimImpl().runImpl(MF))
return PreservedAnalyses::all();
PreservedAnalyses PA;
PA.preserveSet<CFGAnalyses>();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm working on this sort of thing in aeubanks@91e4939. We can port passes in the meantime though.

Question: can an MIR function pass ever modify the corresponding IR function? Module MIR passes can introduce dummy IR functions corresponding to new (e.g. outlined) MIR functions, but that's different.

@paperchalice paperchalice force-pushed the NPM/CodeGen/dead-mi-elimination branch from 40a715c to a788906 Compare February 6, 2024 09:12
@paperchalice paperchalice merged commit c9fd738 into llvm:main Feb 6, 2024
3 of 4 checks passed
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.

None yet

4 participants