[PATCH] D67687: [NewPM][CodeGen] Introduce machine pass and machine pass manager

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

>> You mentioned
>> 
>>> Because their data structures are separated. Logically they are representations of the same/similar thing at different abstraction level.
>> 
>> in the email thread. Do you mean that there should be some sort of `MachineModule`, instead of `Module` corresponding to both IR and MIR?
> 
> By saying "their data structures are separated" I meant that modifying/transforming MIR does not cause any change to IR. MIR does not belong to IR class hierarchy. It is not like changing an IR loop also meaning changing its containing IR function or module.
> 
> Also, in MIR, there is basically one kind of IR unit (in NPM terminology): machine function. IR has many kinds of IR units: basic block(relatively small scope not so useful for optimization), loop, function, cgscc, module.
> 
> In the MIR pipeline (what this patch is about), the IR is not changed and not ran. So all IR analyses results are preserved in MIR pipeline. Modifying MIR could only invalidate MIR analysis results but not IR analyses results.
> 
> Based on the above, the requirement for MIR pipeline about IR is that 1) we don't run any IR transformation pass; 2) we ran machine pass initiated IR analyses; 3) we could not invalidate IR analyses.

When you say we can't invalidate IR analyses, do you mean we need to assert that any IR analyses that a pass tries to get must not be invalidated? Does that require something like https://reviews.llvm.org/D72893?
Also, something like `MachineOutliner::createOutlinedFunction()` does create a new `Function` in order to create a `MachineFunction`, which seems like it could invalidate IR analyses.

> Defining a proxy class does not do any actual work(the invalidation part is not needed, the rest are just querying the delegated pass manager) although it is definitely conceptually appealing. Likewise, a potential adaptor class (maybe only `ModuleToMachineFunctionPassAdaptor` as you mentioned) is possible but other than iterating MIR functions from an IR module, it could not do anything else that an adaptor class is supposed to do like aggregating passes preserved analyses and report it back to containing pass manager. The MachineFunctionPassManager is roughly `ModuleToMachineFunctionPassAdaptor` + `PassManager<MachineFunction, MachineFunctionAnalysisManager>` combined.

I don't understand "it could not do anything else that an adaptor class is supposed to do like aggregating passes preserved analyses and report it back to containing pass manager", can you explain that more?

> In conclusion, it is possible to use proxy and adaptor classes here and make things work. But IMHO it is unnecessary coupling into IR pass manager infra. Although it is appealing conceptually, it could also cause confusion. Actually, without the `doInitialization`/`doFinalization` stuff here, all existing code is necessary even if we changed to use proxy and adaptor classes.
> 
> Please let me know if this makes any sense to you. :-)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67687