[PATCH] D64188: [NewPM] Port the MIR Printing pass to new pass manager.

Charles Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 30 15:02:28 PDT 2019


czhang marked an inline comment as done.
czhang added a comment.

In D64188#1606039 <https://reviews.llvm.org/D64188#1606039>, @fedor.sergeev wrote:

> I'm not familiar with MIR printing usecases, and there are no tests for MIR printer.
>  Yet if MIR printing pass is to be applied to the Module only then there is no need to make it a MachineFunction pass, just do everything in a single run(Module...) method.
>
> I would expect to see a registration of this pass in PassRegistry.def as a module pass. Perhaps named as print-mir?
>
> Also, this patch should definitely have some dependencies on prior work introducing MachineFunction as IRUnit etc...


I'm not so clear on the use case of the MIR printing pass either, but for sure, llc uses it. To be safe, I just followed how the legacy pass manager did things to try and keep the original intent the same. For example, I did not register the pass in the pass registry, because it is not actually part of the created pass pipeline in the legacy pass manager in llc. There is no way to specify print-mir as part of -run-pass, perhaps because it is already triggered by other command flags. So since MIR printing is done separately in llc, I have followed that convention and done the same in my llc patch.



================
Comment at: llvm/lib/CodeGen/MIRPrintingPass.cpp:26-30
+  // FIXME: Inefficient. Should only make this adaptor once. Put it in
+  // the class and construct it the first time.
+  return createModuleToFunctionPassAdaptor(
+             createFunctionToMachineFunctionPassAdaptor(*this))
+      .run(M, MAM);
----------------
fedor.sergeev wrote:
> This looks as something artificial.
> If you want to iterate through machine functions and print them there is no need to use pass manager.
> Just manually iterate and print.
> 
In the legacy pass manager, printing MIR functions is done exactly this way. I suspect the reason is that MIR printing for individual MachineFunctions is needed for debugging. Another reason I followed the design of the legacy pass manager is that it is quite a lot of boiler plate and analysis querying just to iterate over every function of a module, and then querying MachineModuleInfo to get the MachineFunction out of each Function. In fact, since the adaptors abstract exactly that, I followed the established precedent. Perhaps we could abstract the idea of iterating over the machine functions of a module some other way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64188





More information about the llvm-commits mailing list