[CodeGen] Port Machine Function Splitter from ELF to COFF

[TaoPan]

Summary

Unlike ELF that the name of MCSymbol is combined section name and symbol
name, the name of MCSymbol of COFF doesn't include section name, for
distinguishing with non-split MCSymbol, add BBSectionsColdTextPrefix to
name of MCSymbol.

[TaoPan]

Could you please have a review?

[tmsriram]

Could you please give more details about COFF versus ELF differences with regards to

1. DebuInfo Handling - Separating out a function into a separate section requires that debug info use dwarf ranges.
2. CFI handling

if there are any. I am not familiar with COFF. Thanks for doing this!

llvm/lib/CodeGen/BasicBlockSections.cpp

84

Maybe fix the comment too.

[TaoPan]

Fix name of comment of BBSectionsColdTextPrefix

[TaoPan]

Thanks for your review comment!
I don’t have any more detail information about COFF versus ELF difference with regards to DebugInfo and CFI handling.
I used this patch to build my c micro case, workable.
Is it ok that improves this feature by issue driven as it’s default off?
I’ll try to enable this feature to build some complex application, e.g. chromium windows, improve this feature if there are any issues.

llvm/lib/CodeGen/BasicBlockSections.cpp

84

Fixed, thanks!

[snehasish]

IMHO we should not ship the split functions feature targeting COFF without more testing. The machine function splitter relies on basic block sections and we should ensure that the debug information/call frame information generated by basic block sections is appropriate for COFF based consumers. Perhaps we can start by extending the existing CFI and DebugInfo tests in llvm/test/CodeGen/X86/cfi-basic-block-sections-1.ll and llvm/test/DebugInfo/X86/basic-block-sections_1.ll (in a separate patch).

We are also very interested in having this work on windows targets such as Chromium so your initiative here is appreciated!

[TaoPan]

Thanks for your guidance of the existing CFI and DebugInfo tests, I'll add COFF tests.

[TaoPan]

git rebase

[TaoPan]

Use comdat name for split text section

[TaoPan]

Add MFS Windows exception handling testcase

[TaoPan]

Use same triple option format

[TaoPan]

Fix llvm-mc option -mtriple -> -triple

[snehasish]

Hi Tao, since it looks like you are making progress on this! I would suggest adding the Windows exception handling test for basic block sections instead of MFS in particular. It will simplify the test since you don't need profile information and we will get better confidence for the Propeller framework overall. It would be equivalent to this test: llvm/test/CodeGen/X86/basic-block-sections-eh.ll

[TaoPan]

Thanks Snehasish! I got the simplicity and value of Windows exception handling test for basic block sections. Is it ok adding llvm/test/CodeGen/X86/basic-block-sections-wineh.ll in a separate patch, as running the test won't execute the source code in this patch?

[snehasish]

Yes, definitely adding it in a separate patch is a good idea :) Lets submit that patch first, before we submit with this one (otherwise any large binary generated by this patch is probably going to be broken).
It should include the necessary code changes for windows unwind info. I think the code modifications for that change should be in files: llvm/llvm-project/llvm/lib/MC/MCWin64EH.cpp and llvm/llvm-project/llvm/lib/CodeGen/AsmPrinter/WinException.cpp (this one will have changes for C++ programs with exceptions enabled).

llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h

MCSection *SelectSectionForGlobal(const GlobalObject *GO, SectionKind Kind,
                                     const TargetMachine &TM) const override;
 
   void getNameWithPrefix(SmallVectorImpl<char> &OutName, const GlobalValue *GV,
                          const TargetMachine &TM) const override;
 
   MCSection *getSectionForJumpTable(const Function &F,
                                     const TargetMachine &TM) const override;
 
+  MCSection *
+  getSectionForMachineBasicBlock(const Function &F,
+                                 const MachineBasicBlock &MBB,
+                                 const TargetMachine &TM) const override;
+
   /// Emit Obj-C garbage collection and linker options.
   void emitModuleMetadata(MCStreamer &Streamer, Module &M) const override;
 
   MCSection *getStaticCtorSection(unsigned Priority,
                                   const MCSymbol *KeySym) const override;
   MCSection *getStaticDtorSection(unsigned Priority,
                                   const MCSymbol *KeySym) const override;
 

llvm/lib/CodeGen/BasicBlockSections.cpp

 using llvm::SmallSet;
 using llvm::SmallVector;
 using llvm::StringMap;
 using llvm::StringRef;
 using namespace llvm;
 
 // Placing the cold clusters in a separate section mitigates against poor
 // profiles and allows optimizations such as hugepage mapping to be applied at a
 // section granularity. Defaults to ".text.split." which is recognized by lld
 // via the `-z keep-text-section-prefix` flag.
 cl::opt<std::string> llvm::BBSectionsColdTextPrefix(
     "bbsections-cold-text-prefix",
     cl::desc("The text prefix to use for cold basic block clusters"),
-    cl::init(".text.split."), cl::Hidden);
+    cl::init("split"), cl::Hidden);
 
 namespace {
 
 // This struct represents the cluster information for a machine basic block.
 struct BBClusterInfo {
   // MachineBasicBlock ID.
   unsigned MBBNumber;
   // Cluster ID this basic block belongs to.

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

MCSection *TargetLoweringObjectFileELF::getSectionForMachineBasicBlock(
 
   // For cold sections use the .text.split. prefix along with the parent
   // function name. All cold blocks for the same function go to the same
   // section. Similarly all exception blocks are grouped by symbol name
   // under the .text.eh prefix. For regular sections, we either use a unique
   // name, or a unique ID for the section.
   SmallString<128> Name;
   if (MBB.getSectionID() == MBBSectionID::ColdSectionID) {
+    Name += ".text.";
     Name += BBSectionsColdTextPrefix;
+    Name += ".";
     Name += MBB.getParent()->getName();
   } else if (MBB.getSectionID() == MBBSectionID::ExceptionSectionID) {
     Name += ".text.eh.";
     Name += MBB.getParent()->getName();
   } else {
     Name += MBB.getParent()->getSection()->getName();
     if (TM.getUniqueBasicBlockSectionNames()) {
       Name += ".";
MCSection *TargetLoweringObjectFileCOFF::getSectionForJumpTable(
   Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
   unsigned UniqueID = NextUniqueID++;
 
   return getContext().getCOFFSection(
       SecName, Characteristics, Kind, COMDATSymName,
       COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE, UniqueID);
 }
 
+/// Returns a unique section for the given machine basic block.
+MCSection *TargetLoweringObjectFileCOFF::getSectionForMachineBasicBlock(
+    const Function &F, const MachineBasicBlock &MBB,
+    const TargetMachine &TM) const {
+  assert(MBB.isBeginSection() && "Basic block does not start a section!");
+  assert(MBB.getSectionID() == MBBSectionID::ColdSectionID);
+  SmallString<128> Name, GroupName;
+  Name += ".text$";
+  Name += BBSectionsColdTextPrefix;
+  GroupName += MBB.getParent()->getName();
+  GroupName += BBSectionsColdTextPrefix;
+  return getContext().getCOFFSection(Name,
+                                     COFF::IMAGE_SCN_CNT_CODE |
+                                         COFF::IMAGE_SCN_MEM_EXECUTE |
+                                         COFF::IMAGE_SCN_MEM_READ,
+                                     SectionKind::getText(), GroupName,
+                                     COFF::IMAGE_COMDAT_SELECT_NODUPLICATES);
+}
+
 void TargetLoweringObjectFileCOFF::emitModuleMetadata(MCStreamer &Streamer,
                                                       Module &M) const {
   emitLinkerDirectives(Streamer, M);
 
   unsigned Version = 0;
   unsigned Flags = 0;
   StringRef Section;
 

llvm/test/CodeGen/X86/basic-block-sections-cold.ll

 ; Check if basic blocks that don't get unique sections are placed in cold sections.
 ; Basic block with id 1 and 2 must be in the cold section.
 ; RUN: echo '!_Z3bazb' > %t
 ; RUN: echo '!!0' >> %t
 ; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
-; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t -unique-basic-block-section-names -bbsections-cold-text-prefix=".text.unlikely." | FileCheck %s -check-prefix=LINUX-SPLIT
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t -unique-basic-block-section-names -bbsections-cold-text-prefix="unlikely" | FileCheck %s -check-prefix=LINUX-SPLIT
 
 define void @_Z3bazb(i1 zeroext %0) nounwind {
   br i1 %0, label %2, label %4
 
 2:                                                ; preds = %1
   %3 = call i32 @_Z3barv()
   br label %6
 

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

 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -split-machine-functions | FileCheck %s -check-prefix=MFS-DEFAULTS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -split-machine-functions | FileCheck %s -check-prefix=MFS-DEFAULTS-MSVC
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -split-machine-functions -mfs-psi-cutoff=0 -mfs-count-threshold=2000 | FileCheck %s --dump-input=always -check-prefix=MFS-OPTS1
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -split-machine-functions -mfs-psi-cutoff=0 -mfs-count-threshold=2000 | FileCheck %s --dump-input=always -check-prefix=MFS-OPTS1-MSVC
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -split-machine-functions -mfs-psi-cutoff=950000 | FileCheck %s -check-prefix=MFS-OPTS2
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -split-machine-functions -mfs-psi-cutoff=950000 | FileCheck %s -check-prefix=MFS-OPTS2-MSVC
 
 define void @foo1(i1 zeroext %0) nounwind !prof !14 !section_prefix !15 {
-;; Check that cold block is moved to .text.split.
+;; Check that cold block is moved to .text.split for ELF or .text$split for COFF
 ; MFS-DEFAULTS-LABEL: foo1
 ; MFS-DEFAULTS:       .section        .text.split.foo1
+; MFS-DEFAULTS-MSVC:  .section        .text$split
 ; MFS-DEFAULTS-NEXT:  foo1.cold:
 ; MFS-DEFAULTS-NOT:   callq   bar
 ; MFS-DEFAULTS-NEXT:  callq   baz
   br i1 %0, label %2, label %4, !prof !17
 
 2:                                                ; preds = %1
   %3 = call i32 @bar()
   br label %6
6:                                                ; preds = %4, %2
   %7 = tail call i32 @qux()
   ret void
 }
 
 define void @foo4(i1 zeroext %0, i1 zeroext %1) nounwind !prof !20 {
 ;; Check that count threshold works.
 ; MFS-OPTS1-LABEL: foo4
 ; MFS-OPTS1:       .section        .text.split.foo4
+; MFS-OPTS1-MSVC:  .section        .text$split
 ; MFS-OPTS1-NEXT:  foo4.cold:
 ; MFS-OPTS1-NOT:   callq    bar
 ; MFS-OPTS1-NOT:   callq    baz
 ; MFS-OPTS1-NEXT:  callq    bam
   br i1 %0, label %3, label %7, !prof !18
 
 3:
   %4 = call i32 @bar()
12:
   %13 = tail call i32 @qux()
   ret void
 }
 
 define void @foo5(i1 zeroext %0, i1 zeroext %1) nounwind !prof !20 {
 ;; Check that profile summary info cutoff works.
 ; MFS-OPTS2-LABEL: foo5
 ; MFS-OPTS2:       .section        .text.split.foo5
+; MFS-OPTS2-MSVC:  .section        .text$split
 ; MFS-OPTS2-NEXT:       foo5.cold:
 ; MFS-OPTS2-NOT:   callq    bar
 ; MFS-OPTS2-NOT:   callq    baz
 ; MFS-OPTS2-NEXT:  callq    bam
   br i1 %0, label %3, label %7, !prof !21
 
 3:
   %4 = call i32 @bar()
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.
 ; MFS-DEFAULTS-LABEL: foo7
 ; MFS-DEFAULTS:       .section        .text.split.foo7,"ax",@progbits
+; MFS-DEFAULTS-MSVC:  .section        .text$split,"xr"
 ; MFS-DEFAULTS-NEXT:  foo7.cold:
 ; MFS-DEFAULTS-NOT:   callq   _Unwind_Resume
 ; MFS-DEFAULTS:       callq   baz
 entry:
   invoke void @_Z1fv()
           to label %try.cont unwind label %lpad
 
 lpad: