[CodeGen][NewPM] Port MachineCycleInfo to NPM #114745

optimisan · 2024-11-04T07:07:03Z

No description provided.

optimisan · 2024-11-04T07:07:11Z

[CodeGen][NewPM] Port MachineCycleInfo to NPM #114745 : 3 dependent PRs (#114746 , #115434 , #116166 ) 👈 (View in Graphite)
[NFC][CodeGen] Clang format MachineSink.cpp #114027
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

github-actions · 2024-11-04T07:10:25Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvmbot · 2024-11-04T11:14:16Z

@llvm/pr-subscribers-backend-x86

Author: Akshat Oke (optimisan)

Changes

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

7 Files Affected:

(modified) llvm/include/llvm/CodeGen/MachineCycleAnalysis.h (+21)
(modified) llvm/include/llvm/InitializePasses.h (+1-1)
(modified) llvm/include/llvm/Passes/MachinePassRegistry.def (+2-1)
(modified) llvm/lib/CodeGen/CodeGen.cpp (+1-1)
(modified) llvm/lib/CodeGen/MachineCycleAnalysis.cpp (+29-9)
(modified) llvm/lib/Passes/PassBuilder.cpp (+1)
(modified) llvm/test/CodeGen/X86/cycle-info.mir (+2)

diff --git a/llvm/include/llvm/CodeGen/MachineCycleAnalysis.h b/llvm/include/llvm/CodeGen/MachineCycleAnalysis.h
index 1888dd053ce65e..64cf30e6ddf3b8 100644
--- a/llvm/include/llvm/CodeGen/MachineCycleAnalysis.h
+++ b/llvm/include/llvm/CodeGen/MachineCycleAnalysis.h
@@ -16,6 +16,7 @@
 
 #include "llvm/ADT/GenericCycleInfo.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachinePassManager.h"
 #include "llvm/CodeGen/MachineSSAContext.h"
 
 namespace llvm {
@@ -46,6 +47,26 @@ class MachineCycleInfoWrapperPass : public MachineFunctionPass {
 //       version.
 bool isCycleInvariant(const MachineCycle *Cycle, MachineInstr &I);
 
+class MachineCycleAnalysis : public AnalysisInfoMixin<MachineCycleAnalysis> {
+  friend AnalysisInfoMixin<MachineCycleAnalysis>;
+  static AnalysisKey Key;
+
+public:
+  using Result = MachineCycleInfo;
+
+  Result run(MachineFunction &MF, MachineFunctionAnalysisManager &MFAM);
+};
+
+class MachineCycleInfoPrinterPass
+    : public PassInfoMixin<MachineCycleInfoPrinterPass> {
+  raw_ostream &OS;
+
+public:
+  explicit MachineCycleInfoPrinterPass(raw_ostream &OS) : OS(OS) {}
+  PreservedAnalyses run(MachineFunction &MF,
+                        MachineFunctionAnalysisManager &MFAM);
+};
+
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_MACHINECYCLEANALYSIS_H
diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h
index 54c070401ec8a4..b040e7c096d1f5 100644
--- a/llvm/include/llvm/InitializePasses.h
+++ b/llvm/include/llvm/InitializePasses.h
@@ -191,7 +191,7 @@ void initializeMachineCFGPrinterPass(PassRegistry &);
 void initializeMachineCSELegacyPass(PassRegistry &);
 void initializeMachineCombinerPass(PassRegistry &);
 void initializeMachineCopyPropagationPass(PassRegistry &);
-void initializeMachineCycleInfoPrinterPassPass(PassRegistry &);
+void initializeMachineCycleInfoPrinterLegacyPass(PassRegistry &);
 void initializeMachineCycleInfoWrapperPassPass(PassRegistry &);
 void initializeMachineDominanceFrontierPass(PassRegistry &);
 void initializeMachineDominatorTreeWrapperPassPass(PassRegistry &);
diff --git a/llvm/include/llvm/Passes/MachinePassRegistry.def b/llvm/include/llvm/Passes/MachinePassRegistry.def
index 9d12a120ff7ac6..bfe8caba0ce0b3 100644
--- a/llvm/include/llvm/Passes/MachinePassRegistry.def
+++ b/llvm/include/llvm/Passes/MachinePassRegistry.def
@@ -101,6 +101,7 @@ MACHINE_FUNCTION_ANALYSIS("live-vars", LiveVariablesAnalysis())
 MACHINE_FUNCTION_ANALYSIS("machine-block-freq", MachineBlockFrequencyAnalysis())
 MACHINE_FUNCTION_ANALYSIS("machine-branch-prob",
                           MachineBranchProbabilityAnalysis())
+MACHINE_FUNCTION_ANALYSIS("machine-cycles", MachineCycleAnalysis())
 MACHINE_FUNCTION_ANALYSIS("machine-dom-tree", MachineDominatorTreeAnalysis())
 MACHINE_FUNCTION_ANALYSIS("machine-loops", MachineLoopAnalysis())
 MACHINE_FUNCTION_ANALYSIS("machine-opt-remark-emitter",
@@ -151,6 +152,7 @@ MACHINE_FUNCTION_PASS("print<machine-branch-prob>",
 MACHINE_FUNCTION_PASS("print<machine-dom-tree>",
                       MachineDominatorTreePrinterPass(dbgs()))
 MACHINE_FUNCTION_PASS("print<machine-loops>", MachineLoopPrinterPass(dbgs()))
+MACHINE_FUNCTION_PASS("print<machine-cycles>", MachineCycleInfoPrinterPass(dbgs()))
 MACHINE_FUNCTION_PASS("print<machine-post-dom-tree>",
                       MachinePostDominatorTreePrinterPass(dbgs()))
 MACHINE_FUNCTION_PASS("print<slot-indexes>", SlotIndexesPrinterPass(dbgs()))
@@ -241,7 +243,6 @@ DUMMY_MACHINE_FUNCTION_PASS("post-RA-sched", PostRASchedulerPass)
 DUMMY_MACHINE_FUNCTION_PASS("postmisched", PostMachineSchedulerPass)
 DUMMY_MACHINE_FUNCTION_PASS("postra-machine-sink", PostRAMachineSinkingPass)
 DUMMY_MACHINE_FUNCTION_PASS("postrapseudos", ExpandPostRAPseudosPass)
-DUMMY_MACHINE_FUNCTION_PASS("print-machine-cycles", MachineCycleInfoPrinterPass)
 DUMMY_MACHINE_FUNCTION_PASS("print-machine-uniformity", MachineUniformityInfoPrinterPass)
 DUMMY_MACHINE_FUNCTION_PASS("processimpdefs", ProcessImplicitDefsPass)
 DUMMY_MACHINE_FUNCTION_PASS("prologepilog", PrologEpilogInserterPass)
diff --git a/llvm/lib/CodeGen/CodeGen.cpp b/llvm/lib/CodeGen/CodeGen.cpp
index 39fba1d0b527ef..adddb8daaa0e91 100644
--- a/llvm/lib/CodeGen/CodeGen.cpp
+++ b/llvm/lib/CodeGen/CodeGen.cpp
@@ -78,7 +78,7 @@ void llvm::initializeCodeGen(PassRegistry &Registry) {
   initializeMachineCSELegacyPass(Registry);
   initializeMachineCombinerPass(Registry);
   initializeMachineCopyPropagationPass(Registry);
-  initializeMachineCycleInfoPrinterPassPass(Registry);
+  initializeMachineCycleInfoPrinterLegacyPass(Registry);
   initializeMachineCycleInfoWrapperPassPass(Registry);
   initializeMachineDominatorTreeWrapperPassPass(Registry);
   initializeMachineFunctionPrinterPassPass(Registry);
diff --git a/llvm/lib/CodeGen/MachineCycleAnalysis.cpp b/llvm/lib/CodeGen/MachineCycleAnalysis.cpp
index 57f7a098ac1757..6e58439960e26b 100644
--- a/llvm/lib/CodeGen/MachineCycleAnalysis.cpp
+++ b/llvm/lib/CodeGen/MachineCycleAnalysis.cpp
@@ -54,43 +54,63 @@ void MachineCycleInfoWrapperPass::releaseMemory() {
   F = nullptr;
 }
 
+AnalysisKey MachineCycleAnalysis::Key;
+
+MachineCycleInfo
+MachineCycleAnalysis::run(MachineFunction &MF,
+                          MachineFunctionAnalysisManager &MFAM) {
+  MachineCycleInfo MCI;
+  MCI.compute(MF);
+  return MCI;
+}
+
 namespace {
-class MachineCycleInfoPrinterPass : public MachineFunctionPass {
+class MachineCycleInfoPrinterLegacy : public MachineFunctionPass {
 public:
   static char ID;
 
-  MachineCycleInfoPrinterPass();
+  MachineCycleInfoPrinterLegacy();
 
   bool runOnMachineFunction(MachineFunction &F) override;
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 };
 } // namespace
 
-char MachineCycleInfoPrinterPass::ID = 0;
+char MachineCycleInfoPrinterLegacy::ID = 0;
 
-MachineCycleInfoPrinterPass::MachineCycleInfoPrinterPass()
+MachineCycleInfoPrinterLegacy::MachineCycleInfoPrinterLegacy()
     : MachineFunctionPass(ID) {
-  initializeMachineCycleInfoPrinterPassPass(*PassRegistry::getPassRegistry());
+  initializeMachineCycleInfoPrinterLegacyPass(*PassRegistry::getPassRegistry());
 }
 
-INITIALIZE_PASS_BEGIN(MachineCycleInfoPrinterPass, "print-machine-cycles",
+INITIALIZE_PASS_BEGIN(MachineCycleInfoPrinterLegacy, "print-machine-cycles",
                       "Print Machine Cycle Info Analysis", true, true)
 INITIALIZE_PASS_DEPENDENCY(MachineCycleInfoWrapperPass)
-INITIALIZE_PASS_END(MachineCycleInfoPrinterPass, "print-machine-cycles",
+INITIALIZE_PASS_END(MachineCycleInfoPrinterLegacy, "print-machine-cycles",
                     "Print Machine Cycle Info Analysis", true, true)
 
-void MachineCycleInfoPrinterPass::getAnalysisUsage(AnalysisUsage &AU) const {
+void MachineCycleInfoPrinterLegacy::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesAll();
   AU.addRequired<MachineCycleInfoWrapperPass>();
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
-bool MachineCycleInfoPrinterPass::runOnMachineFunction(MachineFunction &F) {
+bool MachineCycleInfoPrinterLegacy::runOnMachineFunction(MachineFunction &F) {
   auto &CI = getAnalysis<MachineCycleInfoWrapperPass>();
   CI.print(errs());
   return false;
 }
 
+PreservedAnalyses
+MachineCycleInfoPrinterPass::run(MachineFunction &MF,
+                                 MachineFunctionAnalysisManager &MFAM) {
+  OS << "MachineCycleInfo for function: " << MF.getName() << "\n";
+
+  auto &MCI = MFAM.getResult<MachineCycleAnalysis>(MF);
+  MCI.print(OS);
+  return PreservedAnalyses::all();
+}
+
 bool llvm::isCycleInvariant(const MachineCycle *Cycle, MachineInstr &I) {
   MachineFunction *MF = I.getParent()->getParent();
   MachineRegisterInfo *MRI = &MF->getRegInfo();
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index a879918005cad8..b9148eae848706 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -104,6 +104,7 @@
 #include "llvm/CodeGen/MachineBlockFrequencyInfo.h"
 #include "llvm/CodeGen/MachineBranchProbabilityInfo.h"
 #include "llvm/CodeGen/MachineCSE.h"
+#include "llvm/CodeGen/MachineCycleAnalysis.h"
 #include "llvm/CodeGen/MachineDominators.h"
 #include "llvm/CodeGen/MachineFunctionAnalysis.h"
 #include "llvm/CodeGen/MachineLICM.h"
diff --git a/llvm/test/CodeGen/X86/cycle-info.mir b/llvm/test/CodeGen/X86/cycle-info.mir
index 358ccb2c5e731d..2523d64110d065 100644
--- a/llvm/test/CodeGen/X86/cycle-info.mir
+++ b/llvm/test/CodeGen/X86/cycle-info.mir
@@ -1,5 +1,7 @@
 # RUN: llc -mtriple=x86_64-unknown-linux-gnu -run-pass=print-machine-cycles -o - %s 2>&1 | FileCheck %s
 
+# RUN: llc -mtriple=x86_64-unknown-linux-gnu -passes="print<machine-cycles>" -o - %s 2>&1 | FileCheck %s
+
 ...
 ---
 # CHECK-LABEL: MachineCycleInfo for function: empty

cdevadas

I guess this PR is for porting MachineCycleAnalysis. The run interface for legacy MachineCycle analysis is missing. The porting for the Print interface seems complete.
Also, no test/runline to validate the MachineCycle analysis in the NPM path. I see only the runline for the print interface.

cdevadas · 2024-11-07T09:34:07Z

llvm/include/llvm/Passes/MachinePassRegistry.def

@@ -151,6 +152,7 @@ MACHINE_FUNCTION_PASS("print<machine-branch-prob>",
 MACHINE_FUNCTION_PASS("print<machine-dom-tree>",
                      MachineDominatorTreePrinterPass(dbgs()))
 MACHINE_FUNCTION_PASS("print<machine-loops>", MachineLoopPrinterPass(dbgs()))
+MACHINE_FUNCTION_PASS("print<machine-cycles>", MachineCycleInfoPrinterPass(dbgs()))


Alphabetical order.

The legacy analysis is already named MachineCycleInfoWrapperPass

The printer also runs the analysis to print it.

arsenm · 2025-02-24T08:19:53Z

llvm/lib/CodeGen/MachineCycleAnalysis.cpp

+PreservedAnalyses
+MachineCycleInfoPrinterPass::run(MachineFunction &MF,
+                                 MachineFunctionAnalysisManager &MFAM) {
+  OS << "MachineCycleInfo for function: " << MF.getName() << "\n";


Suggested change

OS << "MachineCycleInfo for function: " << MF.getName() << "\n";

OS << "MachineCycleInfo for function: " << MF.getName() << '\n';

Do we really need this print at all? Do other passes have it? It will not handle anonymous functions correctly as-is.

Analysis passes have corresponding printer passes which follow the legacy pass' print implementation. Here the behaviour would be the same as calling MachineCycleInfoPrinterLegacy (regarding the anonymous func issue)

I think we should rip all of this out. This can be done generically outside the pass if it's useful

There is one test that checks for cycle info using this printer pass, so I'm not sure.
One alternative to print<machine-cycles> is printing under debug and running a require<machine-cycles> pass.

I mean the pre-printing of the name part, not the whole print

Still not convinced, two things:

Leaving out the MF name pre print would lose the printing information available through minimal effort.

Tests usually do this:

; CHECK: SomeAnalysis for function: someFunction ; CHECK-NEXT: SomeAnalysis for BB %0 is: ... ; CHECK: SomeAnalysis for function: otherFunction ...

For anonymous functions, this prints an empty string. Is any special handling needed?

Still not convinced, two things:

Leaving out the MF name pre print would lose the printing information available through minimal effort.

Tests usually do this:

This should not be the responsibility of every single pass. The pass manager itself can directly print the name of the pass without requiring this boilerplate everywhere

; CHECK: SomeAnalysis for function: someFunction ; CHECK-NEXT: SomeAnalysis for BB %0 is: ... ; CHECK: SomeAnalysis for function: otherFunction ...

For anonymous functions, this prints an empty string. Is any special handling needed?

Yes, that's the problem. printAsOperand handles this case

Alright, got it.
-print-{before|after} options already print MF info so we can remove it from here.

This was referenced Nov 4, 2024

[NFC][CodeGen] Clang format MachineSink.cpp #114027

Merged

[CodeGen] Move EnableSinkAndFold to TargetOptions #114746

Closed

optimisan force-pushed the users/Akshat-Oke/10-30-_codegen_newpm_port_machinecycleinfo_to_npm branch 2 times, most recently from e7e38bc to ada4056 Compare November 4, 2024 10:03

optimisan requested review from paperchalice and cdevadas November 4, 2024 10:18

optimisan marked this pull request as ready for review November 4, 2024 11:13

llvmbot added the backend:X86 label Nov 4, 2024

cdevadas reviewed Nov 7, 2024

View reviewed changes

optimisan force-pushed the users/Akshat-Oke/10-29-_nfc_codegen_clang_format_machinesink.cpp branch from dbedca0 to aeb7c60 Compare November 8, 2024 07:20

optimisan force-pushed the users/Akshat-Oke/10-30-_codegen_newpm_port_machinecycleinfo_to_npm branch from ada4056 to 7972323 Compare November 8, 2024 07:20

optimisan mentioned this pull request Nov 8, 2024

[CodeGen][NewPM] Port MachineSink to NPM #115434

Merged

optimisan force-pushed the users/Akshat-Oke/10-29-_nfc_codegen_clang_format_machinesink.cpp branch from aeb7c60 to c1661a9 Compare November 14, 2024 06:00

optimisan force-pushed the users/Akshat-Oke/10-30-_codegen_newpm_port_machinecycleinfo_to_npm branch from 7972323 to 68bcb36 Compare November 14, 2024 06:00

optimisan mentioned this pull request Nov 14, 2024

[NewPM] Introduce MFAnalysisGetter for a common analysis getter #116166

Closed

optimisan force-pushed the users/Akshat-Oke/10-29-_nfc_codegen_clang_format_machinesink.cpp branch from c1661a9 to 071ad8e Compare November 14, 2024 11:53

optimisan force-pushed the users/Akshat-Oke/10-30-_codegen_newpm_port_machinecycleinfo_to_npm branch from 68bcb36 to be67756 Compare November 14, 2024 11:54

Base automatically changed from users/Akshat-Oke/10-29-_nfc_codegen_clang_format_machinesink.cpp to main November 14, 2024 13:18

optimisan force-pushed the users/Akshat-Oke/10-30-_codegen_newpm_port_machinecycleinfo_to_npm branch 2 times, most recently from 751c8bf to 6236214 Compare November 19, 2024 12:25

optimisan force-pushed the users/Akshat-Oke/10-30-_codegen_newpm_port_machinecycleinfo_to_npm branch from 6236214 to c366c19 Compare February 21, 2025 06:32

optimisan mentioned this pull request Feb 21, 2025

[MachineBasicBlock][NFC] Decouple SplitCriticalEdges from pass manager #128151

Merged

arsenm approved these changes Feb 24, 2025

View reviewed changes

optimisan force-pushed the users/Akshat-Oke/10-30-_codegen_newpm_port_machinecycleinfo_to_npm branch from c366c19 to 365b9e8 Compare February 24, 2025 09:35

optimisan added 3 commits February 25, 2025 08:44

[CodeGen][NewPM] Port MachineCycleInfo to NPM

32ece2a

Alphabetize registry

0d7ecbb

AS

9895428

remove print

264c087

optimisan force-pushed the users/Akshat-Oke/10-30-_codegen_newpm_port_machinecycleinfo_to_npm branch from 365b9e8 to 264c087 Compare February 25, 2025 09:24

optimisan merged commit 69c8312 into main Mar 3, 2025
11 checks passed

optimisan deleted the users/Akshat-Oke/10-30-_codegen_newpm_port_machinecycleinfo_to_npm branch March 3, 2025 05:56

jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025

[CodeGen][NewPM] Port MachineCycleInfo to NPM (llvm#114745)

2484907

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[CodeGen][NewPM] Port MachineCycleInfo to NPM #114745

[CodeGen][NewPM] Port MachineCycleInfo to NPM #114745

Uh oh!

optimisan commented Nov 4, 2024

Uh oh!

optimisan commented Nov 4, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Nov 4, 2024 •

edited

Loading

Uh oh!

llvmbot commented Nov 4, 2024

Uh oh!

cdevadas left a comment

Uh oh!

cdevadas Nov 7, 2024

Uh oh!

optimisan Nov 7, 2024

Uh oh!

arsenm Feb 24, 2025

Uh oh!

optimisan Feb 24, 2025

Uh oh!

arsenm Feb 24, 2025

Uh oh!

optimisan Feb 24, 2025

Uh oh!

arsenm Feb 24, 2025

Uh oh!

optimisan Feb 25, 2025

Uh oh!

arsenm Feb 25, 2025

Uh oh!

optimisan Feb 25, 2025

Uh oh!

Uh oh!

Uh oh!

	OS << "MachineCycleInfo for function: " << MF.getName() << "\n";
	OS << "MachineCycleInfo for function: " << MF.getName() << '\n';

[CodeGen][NewPM] Port MachineCycleInfo to NPM #114745

[CodeGen][NewPM] Port MachineCycleInfo to NPM #114745

Uh oh!

Conversation

optimisan commented Nov 4, 2024

Uh oh!

optimisan commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 4, 2024

Uh oh!

cdevadas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

optimisan commented Nov 4, 2024 •

edited

Loading

github-actions bot commented Nov 4, 2024 •

edited

Loading