[PATCH] D58258: [HotColdSplit] Schedule splitting late to fix perf regression

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 14 16:36:55 PST 2019

tejohnson added inline comments.

Comment at: llvm/lib/Passes/PassBuilder.cpp:764
+  // is that this has a higher code size cost than splitting early.
+  if (EnableHotColdSplit && Phase != ThinLTOPhase::PreLink)
+    MPM.addPass(HotColdSplittingPass());
I see in the old PM that splitting is being moved post link for full LTO as well. IMO it should be in roughly the same place for the new PM. I guess the issue here is that there is no indication at this point that we are doing full LTO pre-link vs non-LTO. I would say either keep it pre-link in both PMs for full LTO or maybe better to pass down some indication that we are in the full LTO pre-link and not do it here in that case either (and then add it to buildLTODefaultPipeline as well to get it enabled for full LTO post-link). I'm reviewing another patch that needs to know this as well (D54175) and is adding a "bool LTOPreLink" parameter to this method. So maybe use the same thing?

Comment at: llvm/test/Other/opt-hot-cold-split.ll:4
 ; RUN: opt -mtriple=x86_64-- -Os -hot-cold-split=true -passes='thinlto-pre-link<Os>' -debug-pass-manager < %s -o /dev/null 2>&1 | FileCheck %s -check-prefix=THINLTO-PRELINK-Os
 ; RUN: opt -mtriple=x86_64-- -Os -hot-cold-split=true -passes='thinlto<Os>' -debug-pass-manager < %s -o /dev/null 2>&1 | FileCheck %s -check-prefix=THINLTO-POSTLINK-Os
Can you add a regular LTO postlink invocation as well? That would be --passes='lto<Os'



More information about the llvm-commits mailing list