[PATCH] D29280: Do not verify MachimeDominatorTree if it is not calculated

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added a comment.

>>> Globals with “available_externally” linkage are never emitted into the object file
>> 
>> So there is no sense to run machine code generation for them.
> 
> Ah thanks that makes sense. The only thing I am still wondering about: if no machine passes are run, MachineDominatorTree::verifyAnalysis or MachineDominatorTree::verifyDomTree shouldn't be called in the first place, no?

The relevant code looks as follows:

  bool MachineFunctionPass::runOnFunction(Function &F) {
    // Do not codegen any 'available_externally' functions at all, they have
    // definitions outside the translation unit.
    if (F.hasAvailableExternallyLinkage())
      return false;

So the pass was run but it decided to skip processing of some functions. Pass Manager does not know if the pass actually did not make its job and calls `verifyAnalysis` for it. The latter obviously crashes. Solution in https://reviews.llvm.org/D27190 tried to extend Pass state so that it keeps a flag, whether or not the pass produced result, but it was decided to use a simpler solution.

> Would this change here in combination with moving the allocation of the dominator tree to runOnMachineFunction be enough to skip the verification in cases runOnMachineFunction is not executed?

Dominator tree appears when `runOnMachineFunction` is run and is invalidated when Pass Manager releases the pass by call to `releaseMemory`, it looks like correct domtree lifetime.

> The change to use unique_ptr might be a worthwhile refactoring, but it looks like most of the changes in this patch are related to the change to unique_ptr instead of skipping the dominator tree verification. I think it would make it easier for reviewers if the patch would only contain the changes required to skip the dominator tree verification.

The simple patch was presented in https://reviews.llvm.org/D28767. In subsequent discussion http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20170123/423110.html it was pointed out that dominator tree should not exist if it does not represent any function. This patch is an attempt to add lifetime management of the dominator tree.


https://reviews.llvm.org/D29280