[NewPM][CodeGen] Introduce machine pass and machine pass manager

[ychen]

Summary

Please refer to the RFC here http://lists.llvm.org/pipermail/llvm-dev/2020-July/143309.html

This patch introduces the interfaces of machine passes and machine pass managers.

machine pass could define four methods:
(1) PreservedAnalyses run(MachineFunction &, MachineFunctionAnalysisManager &). Majority of the machine passes use this.
(2) Error doInitialization(Module &, MachineFunctionAnalysisManager &). Passes like AsmPrinter needs a hook to lower/transform global constructs. (invoked before all passes run method)
(3) Error doFinalization(Module &, MachineFunctionAnalysisManager &). Client: PrintMIRPass. This is also for completeness. (invoked after all passes run method)
(4) Error run(Module &, MachineFunctionAnalysisManager &). Client: MachineOutliner, DebugifyMachineModule. I would call this machine module pass which needs a global scope. It is like (2) but subject to pass ordering. Currently, a pass either define this or (1), but not both.

machine pass manger:

• MachineFunctionAnalysisManager: basically AnalysisManager<MachineFunction> augmented with the ability to register and query IR analyses.
• MachineFunctionPassManager: support only two methods, addPass and run.

[ychen]

Pass in instead of manage IR unit analysis managers in MachineFunctionIRAnalysisManager

[ychen]

update

[ychen]

fix a bug

[arsenm]

LGTM with nits

llvm/include/llvm/CodeGen/PassManager.h

58 ↗

(On Diff #233001)

Extra space before (

63 ↗

(On Diff #233001)

Ditto

llvm/unittests/CodeGen/PassManagerTest.cpp

140

Typo unknonwn

145

Typo unknonwn

[ychen]

In D67687#1852023, @arsenm wrote:

LGTM with nits

@arsenm Thanks for the review! It seems some more folks want to chime in.

[ychen]

• Address Matt's comments

[merge_guards_bot]

Unit tests: pass. 62418 tests passed, 0 failed and 845 were skipped.

clang-tidy: fail. clang-tidy found 0 errors and 2 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[aeubanks]

Any update on this?

[ychen]

In D67687#2105569, @aeubanks wrote:

Any update on this?

Thanks for asking. I'm actively sorting out some design issues around this (There are multiple patches to enable preliminary codegen using NPM). I'm planning to send the RFC to the mailing list very soon (by the end of this week hopefully).

[ychen]

• Update

[ychen]

I made a few fixes and added some missing functionality in the last update:

• introduce the machine module pass which runs over an IR module instead of machine function. Clients are on their own to use MMI to do the IR->MIR mapping. This is for passes like MachineOutliner, MachineDebugify. Also added unit test case for this.
• add PassInstrumentation support
• add RequireCodeGenSCCOrder (now only a TODO) which is needed for IPRA.
• simplify SFINAE to use is_detected

@arsenm does this version look good to you?

[arsenm]

llvm/lib/CodeGen/MachinePassManager.cpp

36

Braces

54

No else after return

69

Braces

[arsenm]

llvm/lib/CodeGen/MachinePassManager.cpp

34

assert(!RequireCodeGenSCCOrder && "not implemented")?

71

Single quotes

80

Braecs

84

Braces

[ychen]

Address comments

[aeubanks]

Can you update the commit description to say what's going on?

llvm/include/llvm/CodeGen/MachinePassManager.h

121

Are we still doing the initialization/finalization thing?

[ychen]

In D67687#2156911, @aeubanks wrote:

Can you update the commit description to say what's going on?

updated.

llvm/include/llvm/CodeGen/MachinePassManager.h

121

I think we have to keep it until we're certain that the alternatives work well, especially for the use cases like AsmPrinter. But that requires we have at least a partially working pipeline with NPM. How about I add a FIXME? We could visit this later.

[ychen]

ping?

[aeubanks]

After thinking about this some more, I still don't understand why MachineFunctions aren't modeled as contained within a Module and don't really see the benefit of all this custom logic. Looking at some legacy passes like MachineOutliner, it's a ModulePass that finds all machine functions itself through the module.
The passes that you gave as examples that have some sort of doInitialization/doFinalization could just become a module pass (I'm thinking about doing that for objc-arc which has a doInitialization)?
Maybe I'm missing something, but going toward the route of modeling modules as containing machine functions and reusing more of the existing infra and less custom stuff specifically for codegen seems better. Then the only thing you'd have to do is add some pass manager/analysis manager typedefs, create a ModuleToMachineFunctionPassAdaptor, and add some proxies to PassBuilder::crossRegisterProxies.
That would make it less confusing and more consistent than having two different run() methods here.
And it would make it easier to share analyses between the different pipelines, by adding the machine function pass manager as part of the overall module pass manager (e.g. in PassBuilder.cpp).

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?

llvm/include/llvm/CodeGen/MachinePassManager.h

20

doInitialization

"like they are"

llvm/lib/CodeGen/MachinePassManager.cpp

48

moulde -> module

llvm/unittests/CodeGen/PassManagerTest.cpp

89

what about tests for returning an Error in doIntialization/doFinalization?

[ychen]

Thanks for chiming in @aeubanks.

In D67687#2162962, @aeubanks wrote:

After thinking about this some more, I still don't understand why MachineFunctions aren't modeled as contained within a Module and don't really see the benefit of all this custom logic. Looking at some legacy passes like MachineOutliner, it's a ModulePass that finds all machine functions itself through the module.

MachineOutliner doesn't really need to be a module pass i.e. working on a Module. It just needs a view of all machine functions. Considering it is a rare need and the legacy pass manager does not support this directly. Using ModulePass is a workaround.

The passes that you gave as examples that have some sort of doInitialization/doFinalization could just become a module pass (I'm thinking about doing that for objc-arc which has a doInitialization)?

I think very likely yes. But as I mentioned in the thread, they have some differences in when they are got called. I'm just being defensive here that defining doInitialization/doFinalization in the initial porting phase. If afterward, we have a lot of things working and doInitialization/doFinalization turns out to be not needed, we could remove them easily. But still, I'm open to not keeping it if that's the consensus.

Maybe I'm missing something, but going toward the route of modeling modules as containing machine functions and reusing more of the existing infra and less custom stuff specifically for codegen seems better. Then the only thing you'd have to do is add some pass manager/analysis manager typedefs, create a ModuleToMachineFunctionPassAdaptor, and add some proxies to PassBuilder::crossRegisterProxies.
That would make it less confusing and more consistent than having two different run() methods here.
And it would make it easier to share analyses between the different pipelines, by adding the machine function pass manager as part of the overall module pass manager (e.g. in PassBuilder.cpp).

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.

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.

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. :-)

[arsenm]

In D67687#2163365, @ychen wrote:

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.

This statement sounds too broad to me. For example, there may not be a 1:1 correspondence from MachineBasicBlock back to an IR BasicBlock, so any control flow analyses would be useless

[ychen]

In D67687#2164285, @arsenm wrote:

In D67687#2163365, @ychen wrote:

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.

This statement sounds too broad to me. For example, there may not be a 1:1 correspondence from MachineBasicBlock back to an IR BasicBlock, so any control flow analyses would be useless

Agree. But this does not contradict what I said? Do you mean this (So all IR analyses results are preserved in MIR pipeline.) is broad?

[aeubanks]

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. :-)

[ychen]

In D67687#2164806, @aeubanks wrote:

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.

I think it is a special case. IIUC, MachineOutliner is the last machine pass before emitting and it is intended. So even if it is changing IR or invalidating IR analysis, it is not affect anything. If it is paranoid, it could choose to keep IR analyses up-to-date manually if not already.

If passes like MachineOutliner become very common (which I doubt), we should probably keep the existing design and let the pass manually handle IR analyses invaliation.

I gave some thoughts about a potential proxy class between machine function and these IR units. It is kind of hard to implement because you don't know before-hand the implication on IR analyses when MIR analyses is invalidated. The reason is, still, MIR and IR has no much concrete (for example, containing) relationship. If you look at the first few lines of comment above OuterAnalysisManagerProxy/InnerAnalysisManagerProxy, the relationship is required.

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?

Let say, ModuleToMachineFunctionPassAdaptor which is a module pass. We could use it in two ways: 1) wrap every machine pass in it and run the codegen process as a module pass pipeline (similar goes for a hypothetical FunctionToMachineFunctionPassAdaptor); 2) make the machine pass manager a module pass and insert it in a module pass pipeline.
For 1) I think the adaptor does not do much other than calling the contained machine pass. As a module pass, it does not modify module, it does not invalidate any IR analyses.
For 2) ModuleToMachineFunctionPassAdaptor is almost just a one-time use when setting up the codegen in BackendUtils.cpp/llc/LTO.cpp etc.. It is not generally useful. So we could merge it with the PassManager<MachineFunction, MachineFunctionAnalysisManager> class. I guess you meant this, right?

WDYT?

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. :-)

[ychen]

Added @craig.topper if he has any general inputs.

[ychen]

Based on the feedbacks from @aeubanks @arsenm @craig.topper , I'll

• add tests for doing the initialization/finalization. (@aeubanks if you're ok with keeping this, if not, please let me know.)
• move the testing of target IR passes from driven by llc to opt. This is also in line with what legacy PM does.
• the pipeline runs all passes on each function / machine function at a time and then the machine function is deleted. This is also what legacy PM does but I missed in the patch.

Suppose we're codegening a TU containing two functions: foo, bar. And the pipeline has two machine passes: pass1, pass2. The order of execution is pass1(on foo), pass2(on foo), <delete>(foo), pass1(on bar), pass2(on bar), <delete>(bar).

If machine module passes like MachineOutiner pass is added into the two-pass pipeline in the middle: pass1, mo_pass, pass2. The order of execution is pass1(on foo), pass1(on bar), mo(on TU), pass2(on foo), <delete>(foo), pass2(on bar), <delete>(bar).

[aeubanks]

I think it is a special case. IIUC, MachineOutliner is the last machine pass before emitting and it is intended. So even if it is changing IR or invalidating IR analysis, it is not affect anything. If it is paranoid, it could choose to keep IR analyses up-to-date manually if not already.

If passes like MachineOutliner become very common (which I doubt), we should probably keep the existing design and let the pass manually handle IR analyses invaliation.

I gave some thoughts about a potential proxy class between machine function and these IR units. It is kind of hard to implement because you don't know before-hand the implication on IR analyses when MIR analyses is invalidated. The reason is, still, MIR and IR has no much concrete (for example, containing) relationship. If you look at the first few lines of comment above OuterAnalysisManagerProxy/InnerAnalysisManagerProxy, the relationship is required.

Ok I think you're right about MachineOutliner being special. Craig mentioned that MachineFunctions are emitted one at a time for memory reasons, and I've heard that from other people too. MachineOutliner breaks that so it must be special.

I'm still fuzzy on exactly why a "containing" relationship is required, but maybe I'm belaboring this point too much.

Let say, ModuleToMachineFunctionPassAdaptor which is a module pass. We could use it in two ways: 1) wrap every machine pass in it and run the codegen process as a module pass pipeline (similar goes for a hypothetical FunctionToMachineFunctionPassAdaptor); 2) make the machine pass manager a module pass and insert it in a module pass pipeline.
For 1) I think the adaptor does not do much other than calling the contained machine pass. As a module pass, it does not modify module, it does not invalidate any IR analyses.
For 2) ModuleToMachineFunctionPassAdaptor is almost just a one-time use when setting up the codegen in BackendUtils.cpp/llc/LTO.cpp etc.. It is not generally useful. So we could merge it with the PassManager<MachineFunction, MachineFunctionAnalysisManager> class. I guess you meant this, right?

WDYT?

I *think* what you mean by "we could merge it with the PassManager<MachineFunction, MachineFunctionAnalysisManager> class" is what I wanted to say? Putting a pass manager in an adaptor is the normal way of doing things, like at [1].
An adaptor would help with things like https://reviews.llvm.org/D82698.

But maybe since the machine function pass managers returns an Error, it doesn't make as much sense to be able to compose it as a module pass.

[1]: https://github.com/llvm/llvm-project/blob/724bf4ee23a3993689d55afb2845949e05c83b1c/llvm/lib/Passes/PassBuilder.cpp#L792

I think I'm slightly more on board with this than before, although I would like somebody like asbirlea to also take a look.

[asbirlea]

IIRC the MIR pipeline is essentially flat (vs the IR one), so at this point it's probably fine to not have the adaptor complexity. However, if this changes to become more than a one time use, then perhaps the Module-Function hierarchy makes sense.
Let me ask this differently, if the CodegenNPM introduced the ModuleToFunctionPassAdaptor now, and it was only used during codegen set-up (assuming negligible overhead for this complexity), is it possible it will be useful long-term in case there are changes in the current restrictions?

It would be very helpful if you added an overall comment explanation (or adding to the documentation) on the decision of which parts of the IR NPM infra is used in the CodegenNPM.

llvm/include/llvm/CodeGen/MachinePassManager.h

46

s/A/An

llvm/lib/CodeGen/MachinePassManager.cpp

36

I'm assuming this and other places meant remove braces for single instruction for/if?

[arsenm]

llvm/lib/CodeGen/MachinePassManager.cpp

36

I meant add for multiline

[ychen]

In D67687#2170494, @asbirlea wrote:

IIRC the MIR pipeline is essentially flat (vs the IR one), so at this point it's probably fine to not have the adaptor complexity. However, if this changes to become more than a one time use, then perhaps the Module-Function hierarchy makes sense.
Let me ask this differently, if the CodegenNPM introduced the ModuleToFunctionPassAdaptor now, and it was only used during codegen set-up (assuming negligible overhead for this complexity), is it possible it will be useful long-term in case there are changes in the current restrictions?

TBH, I don't think the adaptor class would be useful for cases other than codegen setup, because a partial MIR pipeline seems not very useful to be run as part of an IR pipeline. The only other place, I could think of, the adaptor could be useful is to concatenate the pre-codengen IR pipeline with the MIR pipeline. But similar to the possibility of combining opt-pipeline with codegen pipeline, I'm not sure it brings actual benefits other than aesthetics.

It would be very helpful if you added an overall comment explanation (or adding to the documentation) on the decision of which parts of the IR NPM infra is used in the CodegenNPM.

Sure, will do.

[asbirlea]

In D67687#2170879, @ychen wrote:

In D67687#2170494, @asbirlea wrote:

IIRC the MIR pipeline is essentially flat (vs the IR one), so at this point it's probably fine to not have the adaptor complexity. However, if this changes to become more than a one time use, then perhaps the Module-Function hierarchy makes sense.
Let me ask this differently, if the CodegenNPM introduced the ModuleToFunctionPassAdaptor now, and it was only used during codegen set-up (assuming negligible overhead for this complexity), is it possible it will be useful long-term in case there are changes in the current restrictions?

TBH, I don't think the adaptor class would be useful for cases other than codegen setup, because a partial MIR pipeline seems not very useful to be run as part of an IR pipeline. The only other place, I could think of, the adaptor could be useful is to concatenate the pre-codengen IR pipeline with the MIR pipeline. But similar to the possibility of combining opt-pipeline with codegen pipeline, I'm not sure it brings actual benefits other than aesthetics.

Thank you for clarifying. Let's move forward with the current design and document it to keep a record in case it needs to be revisited in the future. It doesn't sound like it will be the case at this point, but it's nice to have the doc to point to :-).

It would be very helpful if you added an overall comment explanation (or adding to the documentation) on the decision of which parts of the IR NPM infra is used in the CodegenNPM.

Sure, will do.

llvm/lib/CodeGen/MachinePassManager.cpp

36

Ah I see, I didn't see this in the styleguide. I always favored not having the braces for single statement if it doesn't affect readability, but either works here I guess.

[ychen]

• Add unit tests for doInit/doFinal and machine module pass, also checks for pass order.
• Address other comments.
• D85168 add support testing codegen IR passes with opt
• D83612 has test cases showing that FreeMachineFunction is called properly.

[asbirlea]

llvm/include/llvm/CodeGen/MachinePassManager.h

20

s/doInitilization/doInitialization

73

doxygen comment here and below?

llvm/lib/CodeGen/MachinePassManager.cpp

45

I think it's clearer if this is a do{}while(true) with the Idx and Size initialization outside the loop.

65

Use Idx directly below?

76

for (;Begin != Idx; ++Begin) {
  ... Passes[Begin]
...

[aeubanks]

LGTM

[ychen]

• Beef up comments, use doxygen format and move them to proper places
• Address other comments

[ychen]

fix the laying issue.

• change buildCodeGenPipeline signature to take pass manager references as inputs.

[ychen]

@dblaikie The laying issue in 31ecf8d29d81d196374a562c6d2bd2c25a62861e is addressed here. Do you mind taking another look? Thanks!

[dblaikie]

In D67687#2452438, @ychen wrote:

@dblaikie The laying issue in 31ecf8d29d81d196374a562c6d2bd2c25a62861e is addressed here. Do you mind taking another look? Thanks!

The last delta on the patch is fairly large - not sure I can absorb it all. If the problematic dependency is no longer present, that's the key thing - I'm happy to take your word for it, if you've checked the included headers/grep'd for references of whichever libraries were incorrectly connected.

[ychen]

In D67687#2453588, @dblaikie wrote:

In D67687#2452438, @ychen wrote:

@dblaikie The laying issue in 31ecf8d29d81d196374a562c6d2bd2c25a62861e is addressed here. Do you mind taking another look? Thanks!

The last delta on the patch is fairly large - not sure I can absorb it all. If the problematic dependency is no longer present, that's the key thing - I'm happy to take your word for it, if you've checked the included headers/grep'd for references of whichever libraries were incorrectly connected.

Yeah, I've checked these. Pretty sure there are no new libraries and their headers are used.

[ychen]

ping?

[aeubanks]

The latest patch looks like it adds a lot more than the original change, is that intended?

[ychen]

In D67687#2466210, @aeubanks wrote:

The latest patch looks like it adds a lot more than the original change, is that intended?

No, that's not intended. You're absolutely right. I somehow reopen the wrong patch. This patch is fine (need no amendment). I should've reopen D83608.

[ychen]

I'll update this to the correct/landed version. Sorry for the noise.

[ychen]

• update to the landed version.

[MaskRay]

LLVMBuild.txt has been deleted. The dependencies are entirely specified by CMakeLists.txt

[ychen]

In D67687#2466403, @MaskRay wrote:

LLVMBuild.txt has been deleted. The dependencies are entirely specified by CMakeLists.txt

This patch was committed when LLVMBuild.txt is still being used. Here is just to put the reviewed patch into its landed's version after I did a wrong patch update above.

llvm/include/llvm/CodeGen/MachinePassManager.h

+//===- PassManager.h --- Pass management for CodeGen ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This header defines the pass manager interface for codegen. The codegen
+// pipeline consists of only machine function passes. There is no container
+// relationship between IR module/function and machine function in terms of pass
+// manager organization. So there is no need for adaptor classes (for example
+// ModuleToMachineFunctionAdaptor). Since invalidation could only happen among
+// machine function passes, there is no proxy classes to handle cross-IR-unit
+// invalidation. IR analysis results are provided for machine function passes by
+// their respective analysis managers such as ModuleAnalysisManager and
+// FunctionAnalysisManager.
+//
+// TODO: Add MachineFunctionProperties support.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CODEGEN_MACHINEPASSMANAGER_H
+#define LLVM_CODEGEN_MACHINEPASSMANAGER_H
+
+#include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/IR/PassManager.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/type_traits.h"
+
+namespace llvm {
+class Module;
+
+extern template class AnalysisManager<MachineFunction>;
+
+/// An AnalysisManager<MachineFunction> that also exposes IR analysis results.
+class MachineFunctionAnalysisManager : public AnalysisManager<MachineFunction> {
+public:
+  using Base = AnalysisManager<MachineFunction>;
+
+  MachineFunctionAnalysisManager() : Base(false), FAM(nullptr), MAM(nullptr) {}
+  MachineFunctionAnalysisManager(FunctionAnalysisManager &FAM,
+                                 ModuleAnalysisManager &MAM,
+                                 bool DebugLogging = false)
+      : Base(DebugLogging), FAM(&FAM), MAM(&MAM) {}
+  MachineFunctionAnalysisManager(MachineFunctionAnalysisManager &&) = default;
+  MachineFunctionAnalysisManager &
+  operator=(MachineFunctionAnalysisManager &&) = default;
+
+  /// Get the result of an analysis pass for a Function.
+  ///
+  /// Runs the analysis if a cached result is not available.
+  template <typename PassT> typename PassT::Result &getResult(Function &F) {
+    return FAM->getResult<PassT>(F);
+  }
+
+  /// Get the cached result of an analysis pass for a Function.
+  ///
+  /// This method never runs the analysis.
+  ///
+  /// \returns null if there is no cached result.
+  template <typename PassT>
+  typename PassT::Result *getCachedResult(Function &F) {
+    return FAM->getCachedResult<PassT>(F);
+  }
+
+  /// Get the result of an analysis pass for a Module.
+  ///
+  /// Runs the analysis if a cached result is not available.
+  template <typename PassT> typename PassT::Result &getResult(Module &M) {
+    return MAM->getResult<PassT>(M);
+  }
+
+  /// Get the cached result of an analysis pass for a Module.
+  ///
+  /// This method never runs the analysis.
+  ///
+  /// \returns null if there is no cached result.
+  template <typename PassT> typename PassT::Result *getCachedResult(Module &M) {
+    return MAM->getCachedResult<PassT>(M);
+  }
+
+  /// Get the result of an analysis pass for a MachineFunction.
+  ///
+  /// Runs the analysis if a cached result is not available.
+  using Base::getResult;
+
+  /// Get the cached result of an analysis pass for a MachineFunction.
+  ///
+  /// This method never runs the analysis.
+  ///
+  /// \returns null if there is no cached result.
+  using Base::getCachedResult;
+
+  // FIXME: Add LoopAnalysisManager or CGSCCAnalysisManager if needed.
+  FunctionAnalysisManager *FAM;
+  ModuleAnalysisManager *MAM;
+};
+
+extern template class PassManager<MachineFunction>;
+
+/// MachineFunctionPassManager adds/removes below features to/from the base
+/// PassManager template instantiation.
+///
+/// - Support passes that implement doInitialization/doFinalization. This is for
+///   machine function passes to work on module level constructs. One such pass
+///   is AsmPrinter.
+///
+/// - Support machine module pass which runs over the module (for example,
+///   MachineOutliner). A machine module pass needs to define the method:
+///
+///   ```Error run(Module &, MachineFunctionAnalysisManager &)```
+///
+///   FIXME: machine module passes still need to define the usual machine
+///          function pass interface, namely,
+///          `PreservedAnalyses run(MachineFunction &,
+///                                 MachineFunctionAnalysisManager &)`
+///          But this interface wouldn't be executed. It is just a placeholder
+///          to satisfy the pass manager type-erased inteface. This
+///          special-casing of machine module pass is due to its limited use
+///          cases and the unnecessary complexity it may bring to the machine
+///          pass manager.
+///
+/// - The base class `run` method is replaced by an alternative `run` method.
+///   See details below.
+///
+/// - Support codegening in the SCC order. Users include interprocedural
+///   register allocation (IPRA).
+class MachineFunctionPassManager
+    : public PassManager<MachineFunction, MachineFunctionAnalysisManager> {
+  using Base = PassManager<MachineFunction, MachineFunctionAnalysisManager>;
+
+public:
+  MachineFunctionPassManager(bool DebugLogging = false,
+                             bool RequireCodeGenSCCOrder = false)
+      : Base(DebugLogging), RequireCodeGenSCCOrder(RequireCodeGenSCCOrder) {}
+  MachineFunctionPassManager(MachineFunctionPassManager &&) = default;
+  MachineFunctionPassManager &
+  operator=(MachineFunctionPassManager &&) = default;
+
+  /// Run machine passes for a Module.
+  ///
+  /// The intended use is to start the codegen pipeline for a Module. The base
+  /// class's `run` method is deliberately hidden by this due to the observation
+  /// that we don't yet have the use cases of compositing two instances of
+  /// machine pass managers, or compositing machine pass managers with other
+  /// types of pass managers.
+  Error run(Module &M, MachineFunctionAnalysisManager &MFAM);
+
+  template <typename PassT> void addPass(PassT &&Pass) {
+    Base::addPass(std::forward<PassT>(Pass));
+    PassConceptT *P = Passes.back().get();
+    addDoInitialization<PassT>(P);
+    addDoFinalization<PassT>(P);
+
+    // Add machine module pass.
+    addRunOnModule<PassT>(P);
+  }
+
+private:
+  template <typename PassT>
+  using has_init_t = decltype(std::declval<PassT &>().doInitialization(
+      std::declval<Module &>(),
+      std::declval<MachineFunctionAnalysisManager &>()));
+
+  template <typename PassT>
+  std::enable_if_t<!is_detected<has_init_t, PassT>::value>
+  addDoInitialization(PassConceptT *Pass) {}
+
+  template <typename PassT>
+  std::enable_if_t<is_detected<has_init_t, PassT>::value>
+  addDoInitialization(PassConceptT *Pass) {
+    using PassModelT =
+        detail::PassModel<MachineFunction, PassT, PreservedAnalyses,
+                          MachineFunctionAnalysisManager>;
+    auto *P = static_cast<PassModelT *>(Pass);
+    InitializationFuncs.emplace_back(
+        [=](Module &M, MachineFunctionAnalysisManager &MFAM) {
+          return P->Pass.doInitialization(M, MFAM);
+        });
+  }
+
+  template <typename PassT>
+  using has_fini_t = decltype(std::declval<PassT &>().doFinalization(
+      std::declval<Module &>(),
+      std::declval<MachineFunctionAnalysisManager &>()));
+
+  template <typename PassT>
+  std::enable_if_t<!is_detected<has_fini_t, PassT>::value>
+  addDoFinalization(PassConceptT *Pass) {}
+
+  template <typename PassT>
+  std::enable_if_t<is_detected<has_fini_t, PassT>::value>
+  addDoFinalization(PassConceptT *Pass) {
+    using PassModelT =
+        detail::PassModel<MachineFunction, PassT, PreservedAnalyses,
+                          MachineFunctionAnalysisManager>;
+    auto *P = static_cast<PassModelT *>(Pass);
+    FinalizationFuncs.emplace_back(
+        [=](Module &M, MachineFunctionAnalysisManager &MFAM) {
+          return P->Pass.doFinalization(M, MFAM);
+        });
+  }
+
+  template <typename PassT>
+  using is_machine_module_pass_t = decltype(std::declval<PassT &>().run(
+      std::declval<Module &>(),
+      std::declval<MachineFunctionAnalysisManager &>()));
+
+  template <typename PassT>
+  using is_machine_function_pass_t = decltype(std::declval<PassT &>().run(
+      std::declval<MachineFunction &>(),
+      std::declval<MachineFunctionAnalysisManager &>()));
+
+  template <typename PassT>
+  std::enable_if_t<!is_detected<is_machine_module_pass_t, PassT>::value>
+  addRunOnModule(PassConceptT *Pass) {}
+
+  template <typename PassT>
+  std::enable_if_t<is_detected<is_machine_module_pass_t, PassT>::value>
+  addRunOnModule(PassConceptT *Pass) {
+    static_assert(is_detected<is_machine_function_pass_t, PassT>::value,
+                  "machine module pass needs to define machine function pass "
+                  "api. sorry.");
+
+    using PassModelT =
+        detail::PassModel<MachineFunction, PassT, PreservedAnalyses,
+                          MachineFunctionAnalysisManager>;
+    auto *P = static_cast<PassModelT *>(Pass);
+    MachineModulePasses.emplace(
+        Passes.size() - 1,
+        [=](Module &M, MachineFunctionAnalysisManager &MFAM) {
+          return P->Pass.run(M, MFAM);
+        });
+  }
+
+  using FuncTy = Error(Module &, MachineFunctionAnalysisManager &);
+  SmallVector<llvm::unique_function<FuncTy>, 4> InitializationFuncs;
+  SmallVector<llvm::unique_function<FuncTy>, 4> FinalizationFuncs;
+
+  using PassIndex = decltype(Passes)::size_type;
+  std::map<PassIndex, llvm::unique_function<FuncTy>> MachineModulePasses;
+
+  // Run codegen in the SCC order.
+  bool RequireCodeGenSCCOrder;
+};
+
+} // end namespace llvm
+
+#endif // LLVM_CODEGEN_MACHINEPASSMANAGER_H

llvm/include/llvm/IR/PassManager.h

using PassModelT =
         detail::PassModel<IRUnitT, PassT, PreservedAnalyses, AnalysisManagerT,
                           ExtraArgTs...>;
 
     Passes.emplace_back(new PassModelT(std::move(Pass)));
   }
 
   static bool isRequired() { return true; }
 
-private:
+protected:
   using PassConceptT =
       detail::PassConcept<IRUnitT, AnalysisManagerT, ExtraArgTs...>;
 
   std::vector<std::unique_ptr<PassConceptT>> Passes;
 
   /// Flag indicating whether we should do debug logging.
   bool DebugLogging;
 };

llvm/lib/CodeGen/CMakeLists.txt

add_llvm_component_library(LLVMCodeGen
   MachineLICM.cpp
   MachineLoopInfo.cpp
   MachineLoopUtils.cpp
   MachineModuleInfo.cpp
   MachineModuleInfoImpls.cpp
   MachineOperand.cpp
   MachineOptimizationRemarkEmitter.cpp
   MachineOutliner.cpp
+  MachinePassManager.cpp
   MachinePipeliner.cpp
   MachinePostDominators.cpp
   MachineRegionInfo.cpp
   MachineRegisterInfo.cpp
   MachineScheduler.cpp
   MachineSink.cpp
   MachineSizeOpts.cpp
   MachineSSAUpdater.cpp

llvm/lib/CodeGen/LLVMBuild.txt

 
 [common]
 subdirectories = AsmPrinter SelectionDAG MIRParser GlobalISel
 
 [component_0]
 type = Library
 name = CodeGen
 parent = Libraries
-required_libraries = Analysis BitReader BitWriter Core MC ProfileData Scalar Support Target TransformUtils
+required_libraries = Analysis BitReader BitWriter Core MC Passes ProfileData Scalar Support Target TransformUtils

llvm/lib/CodeGen/MachinePassManager.cpp

+//===---------- MachinePassManager.cpp ------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains the pass management machinery for machine functions.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/MachinePassManager.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/IR/PassManagerImpl.h"
+
+using namespace llvm;
+
+namespace llvm {
+template class AllAnalysesOn<MachineFunction>;
+template class AnalysisManager<MachineFunction>;
+template class PassManager<MachineFunction>;
+
+Error MachineFunctionPassManager::run(Module &M,
+                                      MachineFunctionAnalysisManager &MFAM) {
+  // MachineModuleAnalysis is a module analysis pass that is never invalidated
+  // because we don't run any module pass in codegen pipeline. This is very
+  // important because the codegen state is stored in MMI which is the analysis
+  // result of MachineModuleAnalysis. MMI should not be recomputed.
+  auto &MMI = MFAM.getResult<MachineModuleAnalysis>(M);
+
+  (void)RequireCodeGenSCCOrder;
+  assert(!RequireCodeGenSCCOrder && "not implemented");
+
+  if (DebugLogging) {
+    dbgs() << "Starting " << getTypeName<MachineFunction>()
+           << " pass manager run.\n";
+  }
+
+  for (auto &F : InitializationFuncs) {
+    if (auto Err = F(M, MFAM))
+      return Err;
+  }
+
+  unsigned Idx = 0;
+  size_t Size = Passes.size();
+  do {
+    // Run machine module passes
+    for (; MachineModulePasses.count(Idx) && Idx != Size; ++Idx) {
+      if (DebugLogging)
+        dbgs() << "Running pass: " << Passes[Idx]->name() << " on "
+               << M.getName() << '\n';
+      if (auto Err = MachineModulePasses.at(Idx)(M, MFAM))
+        return Err;
+    }
+
+    // Finish running all passes.
+    if (Idx == Size)
+      break;
+
+    // Run machine function passes
+
+    // Get index range of machine function passes.
+    unsigned Begin = Idx;
+    for (; !MachineModulePasses.count(Idx) && Idx != Size; ++Idx)
+      ;
+
+    for (Function &F : M) {
+      // Do not codegen any 'available_externally' functions at all, they have
+      // definitions outside the translation unit.
+      if (F.hasAvailableExternallyLinkage())
+        continue;
+
+      MachineFunction &MF = MMI.getOrCreateMachineFunction(F);
+      PassInstrumentation PI = MFAM.getResult<PassInstrumentationAnalysis>(MF);
+
+      for (unsigned I = Begin, E = Idx; I != E; ++I) {
+        auto *P = Passes[I].get();
+
+        if (!PI.runBeforePass<MachineFunction>(*P, MF))
+          continue;
+
+        // TODO: EmitSizeRemarks
+        PreservedAnalyses PassPA = P->run(MF, MFAM);
+        PI.runAfterPass(*P, MF);
+        MFAM.invalidate(MF, PassPA);
+      }
+    }
+  } while (true);
+
+  for (auto &F : FinalizationFuncs) {
+    if (auto Err = F(M, MFAM))
+      return Err;
+  }
+
+  if (DebugLogging) {
+    dbgs() << "Finished " << getTypeName<MachineFunction>()
+           << " pass manager run.\n";
+  }
+
+  return Error::success();
+}
+
+} // namespace llvm

llvm/unittests/CodeGen/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
   Analysis
   AsmParser
   AsmPrinter
   CodeGen
   Core
   MC
   MIRParser
+  Passes
   SelectionDAG
   Support
   Target
   )
 
 add_llvm_unittest(CodeGenTests
   AArch64SelectionDAGTest.cpp
   DIEHashTest.cpp
   LowLevelTypeTest.cpp
   LexicalScopesTest.cpp
   MachineInstrBundleIteratorTest.cpp
   MachineInstrTest.cpp
   MachineOperandTest.cpp
+  PassManagerTest.cpp
   ScalableVectorMVTsTest.cpp
   TypeTraitsTest.cpp
   TargetOptionsTest.cpp
   )
 
 add_subdirectory(GlobalISel)

llvm/unittests/CodeGen/PassManagerTest.cpp

+//===- llvm/unittest/CodeGen/PassManager.cpp - PassManager tests ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/CGSCCPassManager.h"
+#include "llvm/Analysis/LoopAnalysisManager.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/CodeGen/MachinePassManager.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Passes/PassBuilder.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
+#include "llvm/Target/TargetMachine.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+class TestFunctionAnalysis : public AnalysisInfoMixin<TestFunctionAnalysis> {
+public:
+  struct Result {
+    Result(int Count) : InstructionCount(Count) {}
+    int InstructionCount;
+  };
+
+  /// Run the analysis pass over the function and return a result.
+  Result run(Function &F, FunctionAnalysisManager &AM) {
+    int Count = 0;
+    for (Function::iterator BBI = F.begin(), BBE = F.end(); BBI != BBE; ++BBI)
+      for (BasicBlock::iterator II = BBI->begin(), IE = BBI->end(); II != IE;
+           ++II)
+        ++Count;
+    return Result(Count);
+  }
+
+private:
+  friend AnalysisInfoMixin<TestFunctionAnalysis>;
+  static AnalysisKey Key;
+};
+
+AnalysisKey TestFunctionAnalysis::Key;
+
+class TestMachineFunctionAnalysis
+    : public AnalysisInfoMixin<TestMachineFunctionAnalysis> {
+public:
+  struct Result {
+    Result(int Count) : InstructionCount(Count) {}
+    int InstructionCount;
+  };
+
+  /// Run the analysis pass over the machine function and return a result.
+  Result run(MachineFunction &MF, MachineFunctionAnalysisManager::Base &AM) {
+    auto &MFAM = static_cast<MachineFunctionAnalysisManager &>(AM);
+    // Query function analysis result.
+    TestFunctionAnalysis::Result &FAR =
+        MFAM.getResult<TestFunctionAnalysis>(MF.getFunction());
+    // + 5
+    return FAR.InstructionCount;
+  }
+
+private:
+  friend AnalysisInfoMixin<TestMachineFunctionAnalysis>;
+  static AnalysisKey Key;
+};
+
+AnalysisKey TestMachineFunctionAnalysis::Key;
+
+const std::string DoInitErrMsg = "doInitialization failed";
+const std::string DoFinalErrMsg = "doFinalization failed";
+
+struct TestMachineFunctionPass : public PassInfoMixin<TestMachineFunctionPass> {
+  TestMachineFunctionPass(int &Count, std::vector<int> &BeforeInitialization,
+                          std::vector<int> &BeforeFinalization,
+                          std::vector<int> &MachineFunctionPassCount)
+      : Count(Count), BeforeInitialization(BeforeInitialization),
+        BeforeFinalization(BeforeFinalization),
+        MachineFunctionPassCount(MachineFunctionPassCount) {}
+
+  Error doInitialization(Module &M, MachineFunctionAnalysisManager &MFAM) {
+    // Force doInitialization fail by starting with big `Count`.
+    if (Count > 10000)
+      return make_error<StringError>(DoInitErrMsg, inconvertibleErrorCode());
+
+    // + 1
+    ++Count;
+    BeforeInitialization.push_back(Count);
+    return Error::success();
+  }
+  Error doFinalization(Module &M, MachineFunctionAnalysisManager &MFAM) {
+    // Force doFinalization fail by starting with big `Count`.
+    if (Count > 1000)
+      return make_error<StringError>(DoFinalErrMsg, inconvertibleErrorCode());
+
+    // + 1
+    ++Count;
+    BeforeFinalization.push_back(Count);
+    return Error::success();
+  }
+
+  PreservedAnalyses run(MachineFunction &MF,
+                        MachineFunctionAnalysisManager &MFAM) {
+    // Query function analysis result.
+    TestFunctionAnalysis::Result &FAR =
+        MFAM.getResult<TestFunctionAnalysis>(MF.getFunction());
+    // 3 + 1 + 1 = 5
+    Count += FAR.InstructionCount;
+
+    // Query module analysis result.
+    MachineModuleInfo &MMI =
+        MFAM.getResult<MachineModuleAnalysis>(*MF.getFunction().getParent());
+    // 1 + 1 + 1 = 3
+    Count += (MMI.getModule() == MF.getFunction().getParent());
+
+    // Query machine function analysis result.
+    TestMachineFunctionAnalysis::Result &MFAR =
+        MFAM.getResult<TestMachineFunctionAnalysis>(MF);
+    // 3 + 1 + 1 = 5
+    Count += MFAR.InstructionCount;
+
+    MachineFunctionPassCount.push_back(Count);
+
+    return PreservedAnalyses::none();
+  }
+
+  int &Count;
+  std::vector<int> &BeforeInitialization;
+  std::vector<int> &BeforeFinalization;
+  std::vector<int> &MachineFunctionPassCount;
+};
+
+struct TestMachineModulePass : public PassInfoMixin<TestMachineModulePass> {
+  TestMachineModulePass(int &Count, std::vector<int> &MachineModulePassCount)
+      : Count(Count), MachineModulePassCount(MachineModulePassCount) {}
+
+  Error run(Module &M, MachineFunctionAnalysisManager &MFAM) {
+    MachineModuleInfo &MMI = MFAM.getResult<MachineModuleAnalysis>(M);
+    // + 1
+    Count += (MMI.getModule() == &M);
+    MachineModulePassCount.push_back(Count);
+    return Error::success();
+  }
+
+  PreservedAnalyses run(MachineFunction &MF,
+                        MachineFunctionAnalysisManager &AM) {
+    llvm_unreachable(
+        "This should never be reached because this is machine module pass");
+  }
+
+  int &Count;
+  std::vector<int> &MachineModulePassCount;
+};
+
+std::unique_ptr<Module> parseIR(LLVMContext &Context, const char *IR) {
+  SMDiagnostic Err;
+  return parseAssemblyString(IR, Err, Context);
+}
+
+class PassManagerTest : public ::testing::Test {
+protected:
+  LLVMContext Context;
+  std::unique_ptr<Module> M;
+  std::unique_ptr<TargetMachine> TM;
+
+public:
+  PassManagerTest()
+      : M(parseIR(Context, "define void @f() {\n"
+                           "entry:\n"
+                           "  call void @g()\n"
+                           "  call void @h()\n"
+                           "  ret void\n"
+                           "}\n"
+                           "define void @g() {\n"
+                           "  ret void\n"
+                           "}\n"
+                           "define void @h() {\n"
+                           "  ret void\n"
+                           "}\n")) {
+    // MachineModuleAnalysis needs a TargetMachine instance.
+    llvm::InitializeAllTargets();
+
+    std::string TripleName = Triple::normalize(sys::getDefaultTargetTriple());
+    std::string Error;
+    const Target *TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
+    if (!TheTarget)
+      return;
+
+    TargetOptions Options;
+    TM.reset(TheTarget->createTargetMachine(TripleName, "", "", Options, None));
+  }
+};
+
+TEST_F(PassManagerTest, Basic) {
+  if (!TM)
+    return;
+
+  LLVMTargetMachine *LLVMTM = static_cast<LLVMTargetMachine *>(TM.get());
+  M->setDataLayout(TM->createDataLayout());
+
+  LoopAnalysisManager LAM(/*DebugLogging=*/true);
+  FunctionAnalysisManager FAM(/*DebugLogging=*/true);
+  CGSCCAnalysisManager CGAM(/*DebugLogging=*/true);
+  ModuleAnalysisManager MAM(/*DebugLogging=*/true);
+  PassBuilder PB(TM.get());
+  PB.registerModuleAnalyses(MAM);
+  PB.registerFunctionAnalyses(FAM);
+  PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
+
+  FAM.registerPass([&] { return TestFunctionAnalysis(); });
+  FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
+  MAM.registerPass([&] { return MachineModuleAnalysis(LLVMTM); });
+  MAM.registerPass([&] { return PassInstrumentationAnalysis(); });
+
+  MachineFunctionAnalysisManager MFAM;
+  {
+    // Test move assignment.
+    MachineFunctionAnalysisManager NestedMFAM(FAM, MAM,
+                                              /*DebugLogging*/ true);
+    NestedMFAM.registerPass([&] { return PassInstrumentationAnalysis(); });
+    NestedMFAM.registerPass([&] { return TestMachineFunctionAnalysis(); });
+    MFAM = std::move(NestedMFAM);
+  }
+
+  int Count = 0;
+  std::vector<int> BeforeInitialization[2];
+  std::vector<int> BeforeFinalization[2];
+  std::vector<int> TestMachineFunctionCount[2];
+  std::vector<int> TestMachineModuleCount[2];
+
+  MachineFunctionPassManager MFPM;
+  {
+    // Test move assignment.
+    MachineFunctionPassManager NestedMFPM(/*DebugLogging*/ true);
+    NestedMFPM.addPass(TestMachineModulePass(Count, TestMachineModuleCount[0]));
+    NestedMFPM.addPass(TestMachineFunctionPass(Count, BeforeInitialization[0],
+                                               BeforeFinalization[0],
+                                               TestMachineFunctionCount[0]));
+    NestedMFPM.addPass(TestMachineModulePass(Count, TestMachineModuleCount[1]));
+    NestedMFPM.addPass(TestMachineFunctionPass(Count, BeforeInitialization[1],
+                                               BeforeFinalization[1],
+                                               TestMachineFunctionCount[1]));
+    MFPM = std::move(NestedMFPM);
+  }
+
+  ASSERT_FALSE(errorToBool(MFPM.run(*M, MFAM)));
+
+  // Check first machine module pass
+  EXPECT_EQ(1u, TestMachineModuleCount[0].size());
+  EXPECT_EQ(3, TestMachineModuleCount[0][0]);
+
+  // Check first machine function pass
+  EXPECT_EQ(1u, BeforeInitialization[0].size());
+  EXPECT_EQ(1, BeforeInitialization[0][0]);
+  EXPECT_EQ(3u, TestMachineFunctionCount[0].size());
+  EXPECT_EQ(10, TestMachineFunctionCount[0][0]);
+  EXPECT_EQ(13, TestMachineFunctionCount[0][1]);
+  EXPECT_EQ(16, TestMachineFunctionCount[0][2]);
+  EXPECT_EQ(1u, BeforeFinalization[0].size());
+  EXPECT_EQ(31, BeforeFinalization[0][0]);
+
+  // Check second machine module pass
+  EXPECT_EQ(1u, TestMachineModuleCount[1].size());
+  EXPECT_EQ(17, TestMachineModuleCount[1][0]);
+
+  // Check second machine function pass
+  EXPECT_EQ(1u, BeforeInitialization[1].size());
+  EXPECT_EQ(2, BeforeInitialization[1][0]);
+  EXPECT_EQ(3u, TestMachineFunctionCount[1].size());
+  EXPECT_EQ(24, TestMachineFunctionCount[1][0]);
+  EXPECT_EQ(27, TestMachineFunctionCount[1][1]);
+  EXPECT_EQ(30, TestMachineFunctionCount[1][2]);
+  EXPECT_EQ(1u, BeforeFinalization[1].size());
+  EXPECT_EQ(32, BeforeFinalization[1][0]);
+
+  EXPECT_EQ(32, Count);
+
+  // doInitialization returns error
+  Count = 10000;
+  MFPM.addPass(TestMachineFunctionPass(Count, BeforeInitialization[1],
+                                       BeforeFinalization[1],
+                                       TestMachineFunctionCount[1]));
+  std::string Message;
+  llvm::handleAllErrors(MFPM.run(*M, MFAM), [&](llvm::StringError &Error) {
+    Message = Error.getMessage();
+  });
+  EXPECT_EQ(Message, DoInitErrMsg);
+
+  // doFinalization returns error
+  Count = 1000;
+  MFPM.addPass(TestMachineFunctionPass(Count, BeforeInitialization[1],
+                                       BeforeFinalization[1],
+                                       TestMachineFunctionCount[1]));
+  llvm::handleAllErrors(MFPM.run(*M, MFAM), [&](llvm::StringError &Error) {
+    Message = Error.getMessage();
+  });
+  EXPECT_EQ(Message, DoFinalErrMsg);
+}
+
+} // namespace