[PATCH] D133782: LiveIntervals: Allow constructing standalone without a PassManager

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 13 13:43:11 PDT 2022


MatzeB added a comment.

I think it's fair to have a version of the algorithm that is not an analysis pass. It feels a bit odd, that the classes are subclasses of `MachineFunctionPass` but can now (also) be created independently without actually being added to a pass manager, but I guess it avoids a log of refactoring, so okay...



================
Comment at: llvm/include/llvm/CodeGen/LiveIntervals.h:101-102
     LiveIntervals();
+    LiveIntervals(MachineFunction &MF, SlotIndexes *Indexes,
+                  MachineDominatorTree *DT);
     ~LiveIntervals() override;
----------------
This constructor doesn't have a definition. Is it from an early version and you meant to remove it?


================
Comment at: llvm/include/llvm/CodeGen/LiveIntervals.h:105-106
 
+    void init(MachineFunction &MF, SlotIndexes *Indexes,
+              MachineDominatorTree *DT);
+
----------------
This probably deserves a comment, mentioning that it should be used when constructor `LiveIntervals` independently without being an actual AnalysisPass.

I'd prefer `compute` as a name, but no strong opinion.


================
Comment at: llvm/include/llvm/CodeGen/SlotIndexes.h:360
 
+    void calculate(MachineFunction &fn);
     bool runOnMachineFunction(MachineFunction &fn) override;
----------------
Maybe call it `compute` to fit the pattern of the `LiveIntervals` code. Again no strong opinion.


================
Comment at: llvm/lib/CodeGen/LiveIntervals.cpp:120-121
 
-bool LiveIntervals::runOnMachineFunction(MachineFunction &fn) {
-  MF = &fn;
+void LiveIntervals::init(MachineFunction &MF_, SlotIndexes *Indexes_,
+                         MachineDominatorTree *DT_) {
+  MF = &MF_;
----------------
Make `Indexes_` and `DT_` parameters references to indicate that they must not be `nullptr`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133782/new/

https://reviews.llvm.org/D133782



More information about the llvm-commits mailing list