[CodeGen][TargetPassConfig] Add unreachable-mbb-elimination pass explicitly

[evgeny777]

Summary

This allows starting from/stopping at this pass when doing partial pipeline execution

[ychen]

This is an unfortunate limit of legacy pass manager lacking runtime pass instrumentation support. NPM has better support of this.

Does this mean that we run UnreachableMachineBlockElimID twice? This new addPass and the old addRequiredID? Is it safe to remove the addRequiredID case?

[evgeny777]

Does this mean that we run UnreachableMachineBlockElimID twice?

If we execute pipeline normally (e.g llc -O3) then no. We'll pick previously added UnreachableMachineBlockElimID pass when scheduling LiveVariables pass (see PMTopLevelManager::schedulePass and findAnalysisPass)
The only way we can run UnreachableMachineBlockElimID twice is:

llc -O3 -stop-after=unreachable-mbb-elimination foo.ll -o foo.mir
llc -O3 -start-after=unreachable-mbb-elimination foo.mir -o foo.s

In the case above the second command will execute UnreachableMachineBlockElimID again (which is no-op due to absence of dead blocks)

Is it safe to remove the addRequiredID case?

I don't think so. This can cause trouble with pipelines constructed using API calls.

[ychen]

Thanks for the explanation! Could please add a comment like "This allows starting from/stopping at this pass when doing partial pipeline execution"?

llvm/lib/CodeGen/TargetPassConfig.cpp

void TargetPassConfig::addOptimizedRegAlloc() {
   addPass(&ProcessImplicitDefsID, false);
 
   // LiveVariables currently requires pure SSA form.
   //
   // FIXME: Once TwoAddressInstruction pass no longer uses kill flags,
   // LiveVariables can be removed completely, and LiveIntervals can be directly
   // computed. (We still either need to regenerate kill flags after regalloc, or
   // preferably fix the scavenger to not depend on them).
+  // FIXME: UnreachableMachineBlockElim is a dependant pass of LiveVariables.
+  // When LiveVariables is removed this has to be removed/moved either.
+  // Explicit addition of UnreachableMachineBlockElim allows stopping before or
+  // after it with -stop-before/-stop-after.
+  addPass(&UnreachableMachineBlockElimID, false);
   addPass(&LiveVariablesID, false);
 
   // Edge splitting is smarter with machine loop info.
   addPass(&MachineLoopInfoID, false);
   addPass(&PHIEliminationID, false);
 
   // Eventually, we want to run LiveIntervals before PHI elimination.
   if (EarlyLiveIntervals)

llvm/test/CodeGen/AArch64/partial-pipeline-execution.ll

 ; RUN: llc -O3 %s -o %t.s
 ; RUN: llc -O3 -stop-after=atomic-expand %s -o %t.mir
 ; RUN: llc -O3 -start-after=atomic-expand %s -o %t2.s
 
 ; If we add tti pass correctly files should be identical
 ; Otherwise LSR will use default TargetTransformInfo and
 ; optimize the loop differently
 ; RUN: cmp %t.s %t2.s
 
+; Check that we can do the same with unreachable-mbb-elimination pass
+; RUN: llc -O3 -stop-after=unreachable-mbb-elimination %s -o %t-mbb-elim.mir
+; RUN: llc -O3 -start-after=unreachable-mbb-elimination %t-mbb-elim.mir -o %t3.s
+; RUN: cmp %t.s %t3.s
+
 ; ModuleID = 'loop.c'
 source_filename = "loop.c"
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64-none-linux-gnu"
 
 @q = dso_local local_unnamed_addr global i32* null, align 8
 
 ; Function Attrs: nofree norecurse nounwind