[PATCH] D19773: Tweak the ThinLTO pass pipeline

[Justin Bogner via llvm-commits]

Mehdi AMINI via llvm-commits <llvm-commits at lists.llvm.org> writes:
> joker.eph updated this revision to Diff 55734.
> joker.eph added a comment.
>
> Fix typo check for LTO
>
>
> http://reviews.llvm.org/D19773
>
> Files:
>   lib/Transforms/IPO/FunctionAttrs.cpp
>   lib/Transforms/IPO/PassManagerBuilder.cpp
>
> Index: lib/Transforms/IPO/PassManagerBuilder.cpp
> ===================================================================
> --- lib/Transforms/IPO/PassManagerBuilder.cpp
> +++ lib/Transforms/IPO/PassManagerBuilder.cpp
> @@ -242,11 +242,6 @@
>    MPM.add(createTailCallEliminationPass()); // Eliminate tail calls
>    MPM.add(createCFGSimplificationPass());     // Merge & remove BBs
>    MPM.add(createReassociatePass());           // Reassociate expressions
> -  if (PrepareForThinLTO) {
> -    MPM.add(createAggressiveDCEPass());        // Delete dead instructions
> -    addInstructionCombiningPass(MPM);          // Combine silly seq's
> -    return;
> -  }
>    // Rotate Loop - disable header duplication at -Oz
>    MPM.add(createLoopRotatePass(SizeLevel == 2 ? 0 : -1));
>    MPM.add(createLICMPass());                  // Hoist loop invariants
> @@ -399,6 +394,14 @@
>  
>    addFunctionSimplificationPasses(MPM);
>  
> +  // FIXME: This is a HACK! The inliner pass above implicitly creates a CGSCC
> +  // pass manager that we are specifically trying to avoid. To prevent this
> +  // we must insert a no-op module pass to reset the pass manager.
> +  MPM.add(createBarrierNoopPass());
> +
> +  if (!DisableUnitAtATime)
> +    MPM.add(createReversePostOrderFunctionAttrsPass());
> +
>    // If we are planning to perform ThinLTO later, let's not bloat the code with
>    // unrolling/vectorization/... now. We'll first run the inliner + CGSCC passes
>    // during ThinLTO and perform the rest of the optimizations afterward.
> @@ -410,24 +413,6 @@
>      return;
>    }
>  
> -  // FIXME: This is a HACK! The inliner pass above implicitly creates a CGSCC
> -  // pass manager that we are specifically trying to avoid. To prevent this
> -  // we must insert a no-op module pass to reset the pass manager.
> -  MPM.add(createBarrierNoopPass());
> -
> -  // Scheduling LoopVersioningLICM when inlining is over, because after that
> -  // we may see more accurate aliasing. Reason to run this late is that too
> -  // early versioning may prevent further inlining due to increase of code
> -  // size. By placing it just after inlining other optimizations which runs 
> -  // later might get benefit of no-alias assumption in clone loop. 
> -  if (UseLoopVersioningLICM) {
> -    MPM.add(createLoopVersioningLICMPass());    // Do LoopVersioningLICM
> -    MPM.add(createLICMPass());                  // Hoist loop invariants
> -  }
> -
> -  if (!DisableUnitAtATime)
> -    MPM.add(createReversePostOrderFunctionAttrsPass());
> -
>    if (!DisableUnitAtATime && OptLevel > 1 && !PrepareForLTO)
>      // Remove avail extern fns and globals definitions if we aren't
>      // compiling an object file for later LTO. For LTO we want to preserve
> @@ -440,16 +425,20 @@
>      // and saves running remaining passes on the eliminated functions.
>      MPM.add(createEliminateAvailableExternallyPass());
>  
> -  if (PerformThinLTO) {
> -    // Remove dead fns and globals. Removing unreferenced functions could lead
> -    // to more opportunities for globalopt.
> -    MPM.add(createGlobalDCEPass());
> -    MPM.add(createGlobalOptimizerPass());
> -    // Remove dead fns and globals after globalopt.
> -    MPM.add(createGlobalDCEPass());
> -    addFunctionSimplificationPasses(MPM);
> +  // Scheduling LoopVersioningLICM when inlining is over, because after that
> +  // we may see more accurate aliasing. Reason to run this late is that too
> +  // early versioning may prevent further inlining due to increase of code
> +  // size. By placing it just after inlining other optimizations which runs 
> +  // later might get benefit of no-alias assumption in clone loop. 
> +  if (UseLoopVersioningLICM) {
> +    MPM.add(createLoopVersioningLICMPass());    // Do LoopVersioningLICM
> +    MPM.add(createLICMPass());                  // Hoist loop invariants
>    }

You've changed the ordering of functionattrs, elim-avail-extern, and
licm in the non-thinlto pipeline here as well, but you didn't mention it
in the commit message. Could you please update the message to explain
why this makes sense?

>  
> +  if (PerformThinLTO)
> +    // Remove dead fns and globals.
> +    MPM.add(createGlobalOptimizerPass());
> +
>    if (EnableNonLTOGlobalsModRef)
>      // We add a fresh GlobalsModRef run at this point. This is particularly
>      // useful as the above will have inlined, DCE'ed, and function-attr
> Index: lib/Transforms/IPO/FunctionAttrs.cpp
> ===================================================================
> --- lib/Transforms/IPO/FunctionAttrs.cpp
> +++ lib/Transforms/IPO/FunctionAttrs.cpp
> @@ -1153,6 +1153,7 @@
>    void getAnalysisUsage(AnalysisUsage &AU) const override {
>      AU.setPreservesCFG();
>      AU.addRequired<CallGraphWrapperPass>();
> +    AU.addPreserved<CallGraphWrapperPass>();

This seems like an independent fix for a bug that was exposed by these
changes, right? Maybe it should go in independently?

>    }
>  };
>  }
>