[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