[llvm] [PassManager] Support MachineFunctionProperties (PR #83668)

Arthur Eubanks via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 11 10:44:56 PDT 2024


================
@@ -64,6 +64,8 @@ extern llvm::cl::opt<bool> UseNewDbgInfoFormat;
 
 namespace llvm {
 
+class MachineFunction;
----------------
aeubanks wrote:

I agree it would be ideal to move any mention of `MachineFunction` out of `llvm/IR`, but this functionality is tricky to support in a clean way

we'd like for a machine function pass to be able to say what invariants the MIR holds (via `MachineFunctionProperties`) and for the pass manager infra to verify that. we could always tell every pass to verify these properties before and after running. initially I thought this would be a bunch of extra boilerplate, but actually looking at `MachineFunctionPass::get[Required,Set,Cleared]Properties`, maybe this is more feasible than I initially imagined. `Set`/`Cleared` aren't very common, so those passes could just manually set/clear the properties on `MF.getProperties()`. as for `Required`, currently any legacy pass that wants to ensure the incoming `MachineFunction` has certain properties needs to override `getRequiredProperties()`. it might actually be less overall code if we had a helper function to verify properties that passes would call, e.g.

```
diff --git a/llvm/include/llvm/CodeGen/MachinePassManager.h b/llvm/include/llvm/CodeGen/MachinePassManager.h
index 7713c55661cc..669076e47634 100644
--- a/llvm/include/llvm/CodeGen/MachinePassManager.h
+++ b/llvm/include/llvm/CodeGen/MachinePassManager.h
@@ -44,7 +44,20 @@ using MachineFunctionAnalysisManager = AnalysisManager<MachineFunction>;
 /// automatically mixes in \c PassInfoMixin.
 template <typename DerivedT>
 struct MachinePassInfoMixin : public PassInfoMixin<DerivedT> {
-  // TODO: Add MachineFunctionProperties support.
+  void verifyMachineFunctionProperties(MachineFunction&MF, MachineFunctionProperties RequiredProperties) {
+#ifndef NDEBUG
+  if (!MF.getProperties().verifyRequiredProperties(RequiredProperties)) {
+    errs() << "MachineFunctionProperties required by " << DerivedT::name()
+           << " pass are not met by function " << MF.getName() << ".\n"
+           << "Required properties: ";
+    RequiredProperties.print(errs());
+    errs() << "\nCurrent properties: ";
+    MF.getProperties().print(errs());
+    errs() << "\n";
+    llvm_unreachable("MachineFunctionProperties check failed");
+  }
+#endif
+  }
 };
 ```

and then a pass that wants to verify would do so like

```
FooPass::run(MachineFunction&MF, ...) {
  verifyMachineFunctionProperties(MF, MachineFunctionProperties().set(
        MachineFunctionProperties::Property::NoVRegs));
}
```

@arsenm @paperchalice thoughts on this approach?

as for other approaches:

initially I was hoping we might be able to augment the return value (`PreservedAnalyses`) of the pass here (in fact the return value was template parameterized at some point), but we also need to verify before the pass runs, so that doesn't work

I tried templatizing `PassConceptT`, meaning we'd specify it in the `MachineFunctionPassManager` declaration, but it turned out to be harder than I thought for reasons I can't remember now

https://github.com/llvm/llvm-project/pull/83668


More information about the llvm-commits mailing list