[CodeGen] Split out cold exception handling pads.

[snehasish]

Summary

Support for splitting exception handling pads was added in D73739. This
change updates the code to split out exception handling pads if profile
information indicates that they are cold.

[rahmanl]

Wouldn't this cause an error when we have both hot and cold eh pads? We must ensure that same section id is assigned to all landing pads (please refer to BasicBlockSections.cpp::assignSections.

[snehasish]

Extend the check to account of multiple landing pads with difference in hotness.
Add a new test to verify the expected behaviour.

[snehasish]

Yes, this would cause an issue if we had hot and cold landing pads. I've updated the logic to account for this and added a test. PTAL, thanks!

[snehasish]

I tried this patch on an instrumented build of mysql server. With this patch, an additional 28KB of text is split out from hot functions, a small but non-trivial improvement considering the size of the .text.hot section is ~1MB.

[rahmanl]

Thanks.
LGTM with minor nits.

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

133

"Use auto & for values and auto * for pointers unless you need to make a copy."
You might as well consider spelling out the type here since the type is not really obvious from this line.

138

Same as above.

llvm/test/CodeGen/X86/machine-function-splitter.ll

206

nit: You can reuse this for the previous test case (the case where all landing pads are cold). You can still have two landing pads and make one of them configurable with a macro.

hot:
  %2 = call i32 @bar()
  invoke void @_Z1fv()
          to label %exit unwind label %lpad2, !prof ![[PROF]]

Just a suggestion. Using macros could make tests less readable too.

[snehasish]

Spell out types instead of auto.

[snehasish]

Thanks for the review.

llvm/test/CodeGen/X86/machine-function-splitter.ll

206

It's an interesting idea but I had a couple of concerns so I left it as is

• It's really easy to miss the macro since the substitution is for the profile metadata
• We would need to invoke llc again with two version of the macro

[rahmanl]

llvm/test/CodeGen/X86/machine-function-splitter.ll

206

SGTM. Thanks for considering.

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) {
   // blocks. Preserving the order of blocks is essential to retaining decisions
   // made by prior passes such as MachineBlockPlacement.
   MF.RenumberBlocks();
   MF.setBBSectionsType(BasicBlockSection::Preset);
   auto *MBFI = &getAnalysis<MachineBlockFrequencyInfo>();
   auto *PSI = &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
 
   for (auto &MBB : MF) {
-    // FIXME: We retain the entry block and conservatively keep all landing pad
-    // blocks as part of the original function. Once D73739 is submitted, we can
-    // improve the handling of ehpads.
-    if ((MBB.pred_empty() || MBB.isEHPad()))
+    if (MBB.isEntryBlock())
       continue;
     if (isColdBlock(MBB, MBFI, PSI))
       MBB.setSectionID(MBBSectionID::ColdSectionID);
   }
 
   auto Comparator = [](const MachineBasicBlock &X, const MachineBasicBlock &Y) {
     return X.getSectionID().Type < Y.getSectionID().Type;
   };
   llvm::sortBasicBlocksAndUpdateBranches(MF, Comparator);
 
   return true;
 }
 
 void MachineFunctionSplitter::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<MachineModuleInfoWrapperPass>();
   AU.addRequired<MachineBlockFrequencyInfo>();
   AU.addRequired<ProfileSummaryInfoWrapperPass>();
 }
 
 char MachineFunctionSplitter::ID = 0;
 INITIALIZE_PASS(MachineFunctionSplitter, "machine-function-splitter",
                 "Split machine functions using profile information", false,
                 false)
 
 MachineFunctionPass *llvm::createMachineFunctionSplitterPass() {
   return new MachineFunctionSplitter();
 }

llvm/test/CodeGen/X86/machine-function-splitter.ll

4:                                                ; preds = %1
   br label %6
 
 6:                                                ; preds = %4, %2
   %7 = tail call i32 @qux()
   ret void
 }
 
 define i32 @foo7(i1 zeroext %0) personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) !prof !14 {
-;; Check that cold ehpads are not split out.
+;; Check that cold ehpads are split out.
 ; MFS-DEFAULTS-LABEL: foo7
 ; MFS-DEFAULTS:       .section        .text.split.foo7,"ax",@progbits
 ; MFS-DEFAULTS-NEXT:  foo7.cold:
-; MFS-DEFAULTS-NOT:   callq   _Unwind_Resume
 ; MFS-DEFAULTS:       callq   baz
+; MFS-DEFAULTS:       callq   _Unwind_Resume@PLT
 entry:
   invoke void @_Z1fv()
           to label %try.cont unwind label %lpad
 
 lpad:
   %1 = landingpad { i8*, i32 }
           cleanup
           catch i8* bitcast (i8** @_ZTIi to i8*)

 !3 = !{!"TotalCount", i64 10000}
 !4 = !{!"MaxCount", i64 10}
 !5 = !{!"MaxInternalCount", i64 1}
 !6 = !{!"MaxFunctionCount", i64 1000}
 !7 = !{!"NumCounts", i64 3}
 !8 = !{!"NumFunctions", i64 5}
 !9 = !{!"DetailedSummary", !10}
 !10 = !{!11, !12, !13}
 !11 = !{i32 10000, i64 100, i32 1}
 !12 = !{i32 999900, i64 100, i32 1}
 !13 = !{i32 999999, i64 1, i32 2}
 !14 = !{!"function_entry_count", i64 7000}
 !15 = !{!"function_section_prefix", !"hot"}
 !16 = !{!"function_section_prefix", !"unlikely"}
 !17 = !{!"branch_weights", i32 7000, i32 0}
 !18 = !{!"branch_weights", i32 3000, i32 4000}
 !19 = !{!"branch_weights", i32 1000, i32 6000}
 !20 = !{!"function_entry_count", i64 10000}
 !21 = !{!"branch_weights", i32 6000, i32 4000}
 !22 = !{!"branch_weights", i32 80, i32 9920}
 !23 = !{!"function_entry_count", i64 7}