rahmanl created this revision.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
rahmanl updated this revision to Diff 285826.
rahmanl added a comment.
rahmanl updated this revision to Diff 285828.
rahmanl updated this revision to Diff 285830.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
rahmanl edited the summary of this revision.
rahmanl added reviewers: tmsriram, shenhan, efriedma, MaskRay.
rahmanl added a subscriber: amharc.
rahmanl updated this revision to Diff 285926.
rahmanl edited the summary of this revision.
rahmanl updated this revision to Diff 286044.
rahmanl marked 3 inline comments as done.
rahmanl updated this revision to Diff 286046.
rahmanl marked 6 inline comments as done.
rahmanl updated this revision to Diff 288058.
rahmanl marked an inline comment as done.
Herald added subscribers: wenlei, kerbowa, dexonsmith, steven_wu, nhaehnle,
jvesely, arsenm.
Herald added a reviewer: bollu.
rahmanl updated this revision to Diff 288064.
rahmanl marked 7 inline comments as done.
rahmanl edited the summary of this revision.
rahmanl marked 3 inline comments as done.
rahmanl published this revision for review.
Modify clang test.
rahmanl added a comment.
Add clang test.
rahmanl added a comment.
Modify the clang test.
rahmanl added a comment.
Address Snehasish's comments.
rahmanl added a comment.
- Address Snehasish's comments. clang-format.
rahmanl added a comment.
- Address Snehasish's comments.
rahmanl added a comment.
Thanks for the great feedback @snehasishk.
tmsriram added a comment.
- The summary of this patch must be enhanced a little bit more to capture the
highlights.
- Even though you point to the RFC, I think just add a little more.
- I would add that this patch basically un-pollutes the symtab, that is the
greatest contribution to me which helps profilers, debuggers plug and play
Independently, what did we decide about the option? Are we keeping the option?
**You should also include changes to the clang user manual to reflect this,
documentation around -fbasic-block-section=labels can go in the same patch**
rahmanl added a comment.
Address Sri's comments.
rahmanl added a comment.
Address Sri's comments.
================
Comment at: clang/test/CodeGen/basic-block-sections.c:31-33
+// BB_LABELS: .Lfunc_begin0:
+// BB_LABELS: .LBB_END0_0:
+// BB_LABELS: .LBB0_1:
----------------
We need a test where we disasm the native object file and show how the contents
look like. This is good for readability of these sections.
================
Comment at: clang/test/CodeGen/basic-block-sections.c:31-33
+// BB_LABELS: .Lfunc_begin0:
+// BB_LABELS: .LBB_END0_0:
+// BB_LABELS: .LBB0_1:
----------------
tmsriram wrote:
> We need a test where we disasm the native object file and show how the
> contents look like. This is good for readability of these sections.
The test you're envisioning won't look great.
If we want to dis-assemble the object file, we end up just having binary
contents for every .bb_addr_map section.
================
Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:486
+ /// 2nd bit: set if this is a block ending with a tail call.
+ /// 3rd bit: seet if this is an exception handling (EH) pad.
+ /// The remaining bits are zero.
----------------
Typo.
================
Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:481
+ /// Returns the BB info metadata to be emitted in the bb_info section.
+ unsigned getBBInfoMetadata() const;
----------------
Document the metadata format? AFAICT the only to find out now is reading the
code which generates the metadata as unsigned.
================
Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:481
+ /// Returns the BB info metadata to be emitted in the bb_info section.
+ unsigned getBBInfoMetadata() const;
----------------
snehasish wrote:
> Document the metadata format? AFAICT the only to find out now is reading the
> code which generates the metadata as unsigned.
+1 Let's document this heavily where possible:
* Under what options is this section emitted?
* What is captured here?
* Maybe a quick note on why this is used, just one sentence should be fine.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1049
+ emitLabelDifferenceAsULEB128(MBBSymbol, FunctionSymbol);
+ // Emit the basic block size.
+ emitLabelDifferenceAsULEB128(MBB.getEndSymbol(), MBBSymbol);
----------------
Can you add a comment here that basic block size cannot always be derived from
label offsets due to alignment? Isnt that the only reason, otherwise size is
redundant right?
================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1225
+ OutContext);
+ OutStreamer->emitELFSize(CurrentSectionBeginSym, SizeExp);
+ }
----------------
I am surprised we are not already doing this :). This is needed for your patch
right? Otherwise, this needs to go as a separate patch.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1225
+ OutContext);
+ OutStreamer->emitELFSize(CurrentSectionBeginSym, SizeExp);
+ }
----------------
tmsriram wrote:
> I am surprised we are not already doing this :). This is needed for your
> patch right? Otherwise, this needs to go as a separate patch.
What's happening here is that the lambda function emitELFSizeDirective is
removed since we no longer have two use-cases (only sections).
================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1027
+void AsmPrinter::emitBBInfoSection(const MachineFunction &MF) {
+ if (!MF.hasBBLabels())
+ return;
----------------
I have a slight preference to move this check to the callsite so that the
reader does not have to follow it only to realize that their function of
interest does not emit a bb info section. Though I see the convention in the
code is to call it and then return early. Maybe documenting at the callsite is
enough?
================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1027
+void AsmPrinter::emitBBInfoSection(const MachineFunction &MF) {
+ if (!MF.hasBBLabels())
+ return;
----------------
snehasish wrote:
> I have a slight preference to move this check to the callsite so that the
> reader does not have to follow it only to realize that their function of
> interest does not emit a bb info section. Though I see the convention in the
> code is to call it and then return early. Maybe documenting at the callsite
> is enough?
Ya, I like moving the check to the call site. You can assert here instead and
be done.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1317
+ emitBBInfoSection(*MF);
+
----------------
Add a comment here.
================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:73
+ // label.
+ if (MF->hasBBSections() && isBeginSection()) {
SmallString<5> Suffix;
----------------
I am happy this code is blown away! :)
================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:61
+ return ((unsigned)isReturnBlock()) |
+ ((!empty() && TII->isTailCall(back())) << 1) | (isEHPad() << 2);
+}
----------------
If this block is empty should we even be calling this function?
================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:61
+ return ((unsigned)isReturnBlock()) |
+ ((!empty() && TII->isTailCall(back())) << 1) | (isEHPad() << 2);
+}
----------------
snehasish wrote:
> If this block is empty should we even be calling this function?
Yes. An empty basic block is possible:
https://llvm.org/doxygen/AsmPrinter_8cpp_source.html#l03140
================
Comment at: llvm/lib/CodeGen/MachineFunction.cpp:343
}
/// This method iterates over the basic blocks and assigns their IsBeginSection
----------------
Ditto! Happy to see this gone!
================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:957
+
+MCSection *MCObjectFileInfo::getBBAddrMapSection(const MCSection &TextSec)
const {
+ if (Env != IsELF)
----------------
Run clang-format on the patches.
================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:957
+
+MCSection *MCObjectFileInfo::getBBAddrMapSection(const MCSection &TextSec)
const {
+ if (Env != IsELF)
----------------
tmsriram wrote:
> Run clang-format on the patches.
Apparently, this is the formatting chosen by clang-format.
================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:960
+MCSection *MCObjectFileInfo::getBBInfoSection(const MCSection &TextSec) const {
+ if (Env != IsELF)
+ return BBInfoSection;
----------------
Is this just returning the BBInfoSection pointer which is null? If so then
prefer returning nullptr here to make it clear to the reader.
================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:971
+
+ return Ctx->getELFSection(".bb_info", ELF::SHT_PROGBITS, Flags, 0, GroupName,
+ MCSection::NonUniqueID,
----------------
Can we call the section something other than bb_info? For example when
examining the contents of a binary a name like "stack_sizes" provides some
obvious meaning without further investigation. Similarly, can we call this
something like "bb_addr_offset_map"?
================
Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:971
+
+ return Ctx->getELFSection(".bb_info", ELF::SHT_PROGBITS, Flags, 0, GroupName,
+ MCSection::NonUniqueID,
----------------
snehasish wrote:
> Can we call the section something other than bb_info? For example when
> examining the contents of a binary a name like "stack_sizes" provides some
> obvious meaning without further investigation. Similarly, can we call this
> something like "bb_addr_offset_map"?
Great suggestion. I renamed the section to bb_addr_map and also changed the
function names accordingly.
================
Comment at:
llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll:1
+; RUN: llc < %s -mtriple=x86_64-linux -function-sections
-basic-block-sections=labels | FileCheck %s
+
----------------
Can we make the triples that we use consistent across the various tests?
================
Comment at:
llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll:5
+; CHECK: .section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL: _Z3barv:
+; CHECK-NEXT: [[BAR_BEGIN:.+]]:
----------------
Lets align the contents of the check to make it easier to inspect.
================
Comment at:
llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll:15
+
+;; Check we add .bb_info section to a COMDAT group with the corresponding
.text section if such a COMDAT exists.
+; CHECK: .section
.text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
----------------
Consider moving the CHECK lines into the functions themselves to make it clear
which CHECKS correspond to which functions.
This patch introduces the new .bb_addr_map section feature which allows us to
emit the bits needed for mapping binary profiles to basic blocks into a
separate section.
The format of the emitted data is represented as follows. It includes a header
for every function:
| Address of the function | -> 8 bytes (pointer size) |
| Number of basic blocks in this function (>0) | -> ULEB128 |
|
The header is followed by a BB record for every basic block. These records are
ordered in the same order as MachineBasicBlocks are placed in the function.
Each BB Info is structured as follows:
| Offset of the basic block relative to function begin | -> ULEB128
|
| Binary size of the basic block | -> ULEB128
|
| BB metadata | -> ULEB128 [
MBB.isReturn() OR MBB.hasTailCall() << 1 OR MBB.isEHPad() << 2 ] |
|
The new feature will replace the existing "BB labels" functionality with
-basic-block-sections=labels.
The .bb_addr_map section scrubs the specially-encoded BB symbols from the
binary and makes it friendly to profilers and debuggers.
Furthermore, the new feature reduces the binary size overhead from 70% bloat to
only 12%.
For more information and results please refer to the RFC:
https://lists.llvm.org/pipermail/llvm-dev/2020-July/143512.html
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D85408
Files:
clang/docs/UsersManual.rst
clang/test/CodeGen/basic-block-sections.c
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/include/llvm/CodeGen/MachineBasicBlock.h
llvm/include/llvm/CodeGen/MachineFunction.h
llvm/include/llvm/MC/MCObjectFileInfo.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/BasicBlockSections.cpp
llvm/lib/CodeGen/MIRParser/MIRParser.cpp
llvm/lib/CodeGen/MachineBasicBlock.cpp
llvm/lib/CodeGen/MachineFunction.cpp
llvm/lib/MC/MCObjectFileInfo.cpp
llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
llvm/test/CodeGen/X86/basic-block-sections-labels.ll
Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===================================================================
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,29 @@
; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
-define void @_Z3bazb(i1 zeroext) {
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
%2 = alloca i8, align 1
%3 = zext i1 %0 to i8
store i8 %3, i8* %2, align 1
%4 = load i8, i8* %2, align 1
%5 = trunc i8 %4 to i1
- br i1 %5, label %6, label %8
+ br i1 %5, label %6, label %11
-6: ; preds = %1
- %7 = call i32 @_Z3barv()
- br label %10
+6:
+ %7 = invoke i32 @_Z3barv()
+ to label %11 unwind label %9
+ br label %13
-8: ; preds = %1
- %9 = call i32 @_Z3foov()
- br label %10
+9:
+ landingpad { i8*, i32 }
+ catch i8* null
+ br label %13
-10: ; preds = %8, %6
+11:
+ %12 = call i32 @_Z3foov()
+ br label %13
+
+13:
ret void
}
@@ -25,9 +31,31 @@
declare i32 @_Z3foov() #1
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; LINUX-LABELS-LABEL: _Z3bazb:
+; LINUX-LABELS: .Lfunc_begin0:
+; LINUX-LABELS: .LBB_END0_[[L1:[0-9]+]]:
+; LINUX-LABELS: .LBB0_[[L2:[0-9]+]]:
+; LINUX-LABELS: .LBB_END0_[[L2]]:
+; LINUX-LABELS: .LBB0_[[L3:[0-9]+]]:
+; LINUX-LABELS: .LBB_END0_[[L3]]:
+; LINUX-LABELS: .LBB0_[[L4:[0-9]+]]:
+; LINUX-LABELS: .LBB_END0_[[L4]]:
+; LINUX-LABELS: .Lfunc_end0:
+
+; LINUX-LABELS: .section .bb_addr_map,"o",@progbits,.text
+; LINUX-LABELS-NEXT: .quad .Lfunc_begin0
+; LINUX-LABELS-NEXT: .byte 4
+; LINUX-LABELS-NEXT: .uleb128 .Lfunc_begin0-.Lfunc_begin0
+; LINUX-LABELS-NEXT: .uleb128 .LBB_END0_[[L1]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT: .byte 0
+; LINUX-LABELS-NEXT: .uleb128 .LBB0_[[L2]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT: .uleb128 .LBB_END0_[[L2]]-.LBB0_[[L2]]
+; LINUX-LABELS-NEXT: .byte 0
+; LINUX-LABELS-NEXT: .uleb128 .LBB0_[[L3]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT: .uleb128 .LBB_END0_[[L3]]-.LBB0_[[L3]]
+; LINUX-LABELS-NEXT: .byte 1
+; LINUX-LABELS-NEXT: .uleb128 .LBB0_[[L4]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT: .uleb128 .LBB_END0_[[L4]]-.LBB0_[[L4]]
+; LINUX-LABELS-NEXT: .byte 5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+ ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK: .section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL: _Z3barv:
+; CHECK-NEXT: [[BAR_BEGIN:.+]]:
+; CHECK: .section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT: .quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+ %1 = call i32 @_Z4fooTIiET_v()
+ ret i32 %1
+}
+; CHECK: .section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL: _Z3foov:
+; CHECK-NEXT: [[FOO_BEGIN:.+]]:
+; CHECK: .section .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT: .quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+ ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK: .section .text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
+; CHECK-LABEL: _Z4fooTIiET_v:
+; CHECK-NEXT: [[FOOCOMDAT_BEGIN:.+]]:
+; CHECK: .section .bb_addr_map,"Go",@progbits,_Z4fooTIiET_v,comdat,.text._Z4fooTIiET_v{{$}}
+; CHECK-NEXT: .quad [[FOOCOMDAT_BEGIN]]
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===================================================================
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -953,3 +953,21 @@
GroupName, MCSection::NonUniqueID,
cast<MCSymbolELF>(TextSec.getBeginSymbol()));
}
+
+MCSection *
+MCObjectFileInfo::getBBAddrMapSection(const MCSection &TextSec) const {
+ if (Env != IsELF)
+ return nullptr;
+
+ const MCSectionELF &ElfSec = static_cast<const MCSectionELF &>(TextSec);
+ unsigned Flags = ELF::SHF_LINK_ORDER;
+ StringRef GroupName;
+ if (const MCSymbol *Group = ElfSec.getGroup()) {
+ GroupName = Group->getName();
+ Flags |= ELF::SHF_GROUP;
+ }
+
+ return Ctx->getELFSection(".bb_addr_map", ELF::SHT_PROGBITS, Flags, 0,
+ GroupName, MCSection::NonUniqueID,
+ cast<MCSymbolELF>(TextSec.getBeginSymbol()));
+}
Index: llvm/lib/CodeGen/MachineFunction.cpp
===================================================================
--- llvm/lib/CodeGen/MachineFunction.cpp
+++ llvm/lib/CodeGen/MachineFunction.cpp
@@ -341,33 +341,6 @@
MBBNumbering.resize(BlockNo);
}
-/// This is used with -fbasic-block-sections or -fbasicblock-labels option.
-/// A unary encoding of basic block labels is done to keep ".strtab" sizes
-/// small.
-void MachineFunction::createBBLabels() {
- const TargetInstrInfo *TII = getSubtarget().getInstrInfo();
- this->BBSectionsSymbolPrefix.resize(getNumBlockIDs(), 'a');
- for (auto MBBI = begin(), E = end(); MBBI != E; ++MBBI) {
- assert(
- (MBBI->getNumber() >= 0 && MBBI->getNumber() < (int)getNumBlockIDs()) &&
- "BasicBlock number was out of range!");
- // 'a' - Normal block.
- // 'r' - Return block.
- // 'l' - Landing Pad.
- // 'L' - Return and landing pad.
- bool isEHPad = MBBI->isEHPad();
- bool isRetBlock = MBBI->isReturnBlock() && !TII->isTailCall(MBBI->back());
- char type = 'a';
- if (isEHPad && isRetBlock)
- type = 'L';
- else if (isEHPad)
- type = 'l';
- else if (isRetBlock)
- type = 'r';
- BBSectionsSymbolPrefix[MBBI->getNumber()] = type;
- }
-}
-
/// This method iterates over the basic blocks and assigns their IsBeginSection
/// and IsEndSection fields. This must be called after MBB layout is finalized
/// and the SectionID's are assigned to MBBs.
Index: llvm/lib/CodeGen/MachineBasicBlock.cpp
===================================================================
--- llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -55,33 +55,22 @@
MachineBasicBlock::~MachineBasicBlock() {
}
+unsigned MachineBasicBlock::getBBAddrMapMetadata() const {
+ const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();
+ return ((unsigned)isReturnBlock()) |
+ ((!empty() && TII->isTailCall(back())) << 1) | (isEHPad() << 2);
+}
+
/// Return the MCSymbol for this basic block.
MCSymbol *MachineBasicBlock::getSymbol() const {
if (!CachedMCSymbol) {
const MachineFunction *MF = getParent();
MCContext &Ctx = MF->getContext();
- auto Prefix = Ctx.getAsmInfo()->getPrivateLabelPrefix();
- assert(getNumber() >= 0 && "cannot get label for unreachable MBB");
-
- // We emit a non-temporary symbol for every basic block if we have BBLabels
- // or -- with basic block sections -- when a basic block begins a section.
- // With basic block symbols, we use a unary encoding which can
- // compress the symbol names significantly. For basic block sections where
- // this block is the first in a cluster, we use a non-temp descriptive name.
- // Otherwise we fall back to use temp label.
- if (MF->hasBBLabels()) {
- auto Iter = MF->getBBSectionsSymbolPrefix().begin();
- if (getNumber() < 0 ||
- getNumber() >= (int)MF->getBBSectionsSymbolPrefix().size())
- report_fatal_error("Unreachable MBB: " + Twine(getNumber()));
- // The basic blocks for function foo are named a.BB.foo, aa.BB.foo, and
- // so on.
- std::string Prefix(Iter + 1, Iter + getNumber() + 1);
- std::reverse(Prefix.begin(), Prefix.end());
- CachedMCSymbol =
- Ctx.getOrCreateSymbol(Twine(Prefix) + ".BB." + Twine(MF->getName()));
- } else if (MF->hasBBSections() && isBeginSection()) {
+ // We emit a non-temporary symbol -- with a descriptive name -- if it begins
+ // a section (with basic block sections). Otherwise we fall back to use temp
+ // label.
+ if (MF->hasBBSections() && isBeginSection()) {
SmallString<5> Suffix;
if (SectionID == MBBSectionID::ColdSectionID) {
Suffix += ".cold";
@@ -92,6 +81,7 @@
}
CachedMCSymbol = Ctx.getOrCreateSymbol(MF->getName() + Suffix);
} else {
+ auto Prefix = Ctx.getAsmInfo()->getPrivateLabelPrefix();
CachedMCSymbol = Ctx.getOrCreateSymbol(Twine(Prefix) + "BB" +
Twine(MF->getFunctionNumber()) +
"_" + Twine(getNumber()));
Index: llvm/lib/CodeGen/MIRParser/MIRParser.cpp
===================================================================
--- llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ llvm/lib/CodeGen/MIRParser/MIRParser.cpp
@@ -446,10 +446,8 @@
}
// Check Basic Block Section Flags.
if (MF.getTarget().getBBSectionsType() == BasicBlockSection::Labels) {
- MF.createBBLabels();
MF.setBBSectionsType(BasicBlockSection::Labels);
} else if (MF.hasBBSections()) {
- MF.createBBLabels();
MF.assignBeginEndSections();
}
PFS.SM = &SM;
Index: llvm/lib/CodeGen/BasicBlockSections.cpp
===================================================================
--- llvm/lib/CodeGen/BasicBlockSections.cpp
+++ llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -336,7 +336,6 @@
if (BBSectionsType == BasicBlockSection::Labels) {
MF.setBBSectionsType(BBSectionsType);
- MF.createBBLabels();
return true;
}
@@ -346,7 +345,6 @@
FuncBBClusterInfo))
return true;
MF.setBBSectionsType(BBSectionsType);
- MF.createBBLabels();
assignSectionsAndSortBasicBlocks(MF, FuncBBClusterInfo);
return true;
}
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===================================================================
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1023,6 +1023,35 @@
MCConstantExpr::create(FrameOffset, OutContext));
}
+void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
+ assert(MF.hasBBLabels() && ".bb_addr_map section needs BB labels enabled.");
+
+ MCSection *BBAddrMapSection =
+ getObjFileLowering().getBBAddrMapSection(*MF.getSection());
+ if (!BBAddrMapSection)
+ return;
+
+ const MCSymbol *FunctionSymbol = getFunctionBegin();
+
+ OutStreamer->PushSection();
+ OutStreamer->SwitchSection(BBAddrMapSection);
+ OutStreamer->emitSymbolValue(FunctionSymbol, getPointerSize());
+ // Emit the total number of basic blocks in this function.
+ OutStreamer->emitULEB128IntValue(MF.size());
+ // Emit BB Information for each basic block in the funciton.
+ for (const auto &MBB : MF) {
+ const MCSymbol *MBBSymbol =
+ MBB.pred_empty() ? FunctionSymbol : MBB.getSymbol();
+ // Emit the basic block offset.
+ emitLabelDifferenceAsULEB128(MBBSymbol, FunctionSymbol);
+ // Emit the basic block size. When BBs have alignments, their size cannot
+ // always be computed from their offsets.
+ emitLabelDifferenceAsULEB128(MBB.getEndSymbol(), MBBSymbol);
+ OutStreamer->emitULEB128IntValue(MBB.getBBAddrMapMetadata());
+ }
+ OutStreamer->PopSection();
+}
+
void AsmPrinter::emitStackSizeSection(const MachineFunction &MF) {
if (!MF.getTarget().Options.EmitStackSizeSection)
return;
@@ -1174,34 +1203,26 @@
}
// We must emit temporary symbol for the end of this basic block, if either
- // we have BBLabels enabled and we want to emit size directive for the BBs,
- // or if this basic blocks marks the end of a section (except the section
- // containing the entry basic block as the end symbol for that section is
- // CurrentFnEnd).
- if ((MAI->hasDotTypeDotSizeDirective() && MF->hasBBLabels()) ||
- (MBB.isEndSection() && !MBB.sameSection(&MF->front())))
+ // we have BBLabels enabled or if this basic blocks marks the end of a
+ // section (except the section containing the entry basic block as the end
+ // symbol for that section is CurrentFnEnd).
+ if (MF->hasBBLabels() ||
+ (MAI->hasDotTypeDotSizeDirective() && MBB.isEndSection() &&
+ !MBB.sameSection(&MF->front())))
OutStreamer->emitLabel(MBB.getEndSymbol());
- // Helper for emitting the size directive associated with a basic block
- // symbol.
- auto emitELFSizeDirective = [&](MCSymbol *SymForSize) {
- const MCExpr *SizeExp = MCBinaryExpr::createSub(
- MCSymbolRefExpr::create(MBB.getEndSymbol(), OutContext),
- MCSymbolRefExpr::create(SymForSize, OutContext), OutContext);
- OutStreamer->emitELFSize(SymForSize, SizeExp);
- };
-
- // Emit size directive for the size of each basic block, if BBLabels is
- // enabled.
- if (MAI->hasDotTypeDotSizeDirective() && MF->hasBBLabels())
- emitELFSizeDirective(MBB.getSymbol());
-
- // Emit size directive for the size of each basic block section once we
- // get to the end of that section.
if (MBB.isEndSection()) {
+ // The size directive for the section containing the entry block is
+ // handled separately by the function section.
if (!MBB.sameSection(&MF->front())) {
- if (MAI->hasDotTypeDotSizeDirective())
- emitELFSizeDirective(CurrentSectionBeginSym);
+ if (MAI->hasDotTypeDotSizeDirective()) {
+ // Emit the size directive for the basic block section.
+ const MCExpr *SizeExp = MCBinaryExpr::createSub(
+ MCSymbolRefExpr::create(MBB.getEndSymbol(), OutContext),
+ MCSymbolRefExpr::create(CurrentSectionBeginSym, OutContext),
+ OutContext);
+ OutStreamer->emitELFSize(CurrentSectionBeginSym, SizeExp);
+ }
MBBSectionRanges[MBB.getSectionIDNum()] =
MBBSectionRange{CurrentSectionBeginSym, MBB.getEndSymbol()};
}
@@ -1293,6 +1314,11 @@
HI.Handler->endFunction(MF);
}
+ // Emit section containing BB address offsets and their metadata, when
+ // BB labels are requested for this function.
+ if (MF->hasBBLabels())
+ emitBBAddrMapSection(*MF);
+
// Emit section containing stack size metadata.
emitStackSizeSection(*MF);
@@ -1802,7 +1828,7 @@
F.hasFnAttribute("function-instrument") ||
F.hasFnAttribute("xray-instruction-threshold") ||
needFuncLabelsForEHOrDebugInfo(MF) || NeedsLocalForSize ||
- MF.getTarget().Options.EmitStackSizeSection) {
+ MF.getTarget().Options.EmitStackSizeSection || MF.hasBBLabels()) {
CurrentFnBegin = createTempSymbol("func_begin");
if (NeedsLocalForSize)
CurrentFnSymForSize = CurrentFnBegin;
Index: llvm/include/llvm/MC/MCObjectFileInfo.h
===================================================================
--- llvm/include/llvm/MC/MCObjectFileInfo.h
+++ llvm/include/llvm/MC/MCObjectFileInfo.h
@@ -338,6 +338,8 @@
MCSection *getStackSizesSection(const MCSection &TextSec) const;
+ MCSection *getBBAddrMapSection(const MCSection &TextSec) const;
+
// ELF specific sections.
MCSection *getDataRelROSection() const { return DataRelROSection; }
const MCSection *getMergeableConst4Section() const {
Index: llvm/include/llvm/CodeGen/MachineFunction.h
===================================================================
--- llvm/include/llvm/CodeGen/MachineFunction.h
+++ llvm/include/llvm/CodeGen/MachineFunction.h
@@ -504,9 +504,6 @@
void setBBSectionsType(BasicBlockSection V) { BBSectionsType = V; }
- /// Creates basic block Labels for this function.
- void createBBLabels();
-
/// Assign IsBeginSection IsEndSection fields for basic blocks in this
/// function.
void assignBeginEndSections();
Index: llvm/include/llvm/CodeGen/MachineBasicBlock.h
===================================================================
--- llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -478,6 +478,18 @@
/// Returns the MCSymbol marking the end of this basic block.
MCSymbol *getEndSymbol() const;
+ /// Returns the BB metadata to be emitted in the bb_addr_map section, which is
+ /// emitted under "-basic-block-sections=labels". The metadata can be used
+ /// to capture more precise profile information. For instance, the return
+ /// block bit helps to distinguish recursive return edges vs. indirect
+ /// branches.
+ /// The format of the result is described as follows:
+ /// 1st bit (LSB): set if this is a return block (return or tail call).
+ /// 2nd bit: set if this is a block ending with a tail call.
+ /// 3rd bit: set if this is an exception handling (EH) pad.
+ /// The remaining bits are zero.
+ unsigned getBBAddrMapMetadata() const;
+
/// Returns true if this block may have an INLINEASM_BR (overestimate, by
/// checking if any of the successors are indirect targets of any inlineasm_br
/// in the function).
Index: llvm/include/llvm/CodeGen/AsmPrinter.h
===================================================================
--- llvm/include/llvm/CodeGen/AsmPrinter.h
+++ llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -342,6 +342,8 @@
void emitStackSizeSection(const MachineFunction &MF);
+ void emitBBAddrMapSection(const MachineFunction &MF);
+
void emitRemarksSection(remarks::RemarkStreamer &RS);
enum CFIMoveType { CFI_M_None, CFI_M_EH, CFI_M_Debug };
Index: clang/test/CodeGen/basic-block-sections.c
===================================================================
--- clang/test/CodeGen/basic-block-sections.c
+++ clang/test/CodeGen/basic-block-sections.c
@@ -1,12 +1,12 @@
// REQUIRES: x86-registered-target
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -o - < %s | FileCheck %s --check-prefix=PLAIN
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasic-block-sections=all -fbasic-block-sections=none -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -fbasic-block-sections=all -fbasic-block-sections=none -o - < %s | FileCheck %s --check-prefix=PLAIN
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasic-block-sections=labels -o - < %s | FileCheck %s --check-prefix=BB_LABELS
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasic-block-sections=all -o - < %s | FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_ALL
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasic-block-sections=list=%S/Inputs/basic-block-sections.funcnames -o - < %s | FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_LIST
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasic-block-sections=all -funique-basic-block-section-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -fbasic-block-sections=labels -o - < %s | FileCheck %s --check-prefix=BB_LABELS
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -fbasic-block-sections=all -o - < %s | FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_ALL
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -fbasic-block-sections=list=%S/Inputs/basic-block-sections.funcnames -o - < %s | FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_LIST
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -fbasic-block-sections=all -funique-basic-block-section-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
int world(int a) {
if (a > 10)
@@ -28,9 +28,66 @@
//
// BB_LABELS-NOT: section
// BB_LABELS: world:
-// BB_LABELS: a.BB.world:
-// BB_LABELS: aa.BB.world:
-// BB_LABELS: a.BB.another:
+// BB_LABELS: .Lfunc_begin0:
+// BB_LABELS: .LBB_END0_0:
+// BB_LABELS: .LBB0_1:
+// BB_LABELS: .LBB_END0_1:
+// BB_LABELS: .LBB0_3:
+// BB_LABELS: .LBB_END0_3:
+// BB_LABELS: .LBB0_4:
+// BB_LABELS: .LBB_END0_4:
+// BB_LABELS: .LBB0_5:
+// BB_LABELS: .LBB_END0_5:
+// BB_LABELS: .Lfunc_end0:
+//
+// BB_LABELS: .section .bb_info,"o",@progbits,.text
+// BB_LABELS-NEXT: .quad .Lfunc_begin0
+// BB_LABELS-NEXT: .byte 6
+// BB_LABELS-NEXT: .uleb128 .Lfunc_begin0-.Lfunc_begin0
+// BB_LABELS-NEXT: .uleb128 .LBB_END0_0-.Lfunc_begin0
+// BB_LABELS-NEXT: .byte 0
+// BB_LABELS-NEXT: .uleb128 .LBB0_1-.Lfunc_begin0
+// BB_LABELS-NEXT: .uleb128 .LBB_END0_1-.LBB0_1
+// BB_LABELS-NEXT: .byte 0
+// BB_LABELS-NEXT: .uleb128 .LBB0_2-.Lfunc_begin0
+// BB_LABELS-NEXT: .uleb128 .LBB_END0_2-.LBB0_2
+// BB_LABELS-NEXT: .byte 0
+// BB_LABELS-NEXT: .uleb128 .LBB0_3-.Lfunc_begin0
+// BB_LABELS-NEXT: .uleb128 .LBB_END0_3-.LBB0_3
+// BB_LABELS-NEXT: .byte 0
+// BB_LABELS-NEXT: .uleb128 .LBB0_4-.Lfunc_begin0
+// BB_LABELS-NEXT: .uleb128 .LBB_END0_4-.LBB0_4
+// BB_LABELS-NEXT: .byte 0
+// BB_LABELS-NEXT: .uleb128 .LBB0_5-.Lfunc_begin0
+// BB_LABELS-NEXT: .uleb128 .LBB_END0_5-.LBB0_5
+// BB_LABELS-NEXT: .byte 1
+//
+// BB_LABELS: another:
+// BB_LABELS: .Lfunc_begin1:
+// BB_LABELS: .LBB_END1_0:
+// BB_LABELS: .LBB1_1:
+// BB_LABELS: .LBB_END1_1:
+// BB_LABELS: .LBB1_2:
+// BB_LABELS: .LBB_END1_2:
+// BB_LABELS: .LBB1_3:
+// BB_LABELS: .LBB_END1_3:
+// BB_LABELS: .Lfunc_end1:
+//
+// BB_LABELS: .section .bb_info,"o",@progbits,.text
+// BB_LABELS-NEXT: .quad .Lfunc_begin1
+// BB_LABELS-NEXT: .byte 4
+// BB_LABELS-NEXT: .uleb128 .Lfunc_begin1-.Lfunc_begin1
+// BB_LABELS-NEXT: .uleb128 .LBB_END1_0-.Lfunc_begin1
+// BB_LABELS-NEXT: .byte 0
+// BB_LABELS-NEXT: .uleb128 .LBB1_1-.Lfunc_begin1
+// BB_LABELS-NEXT: .uleb128 .LBB_END1_1-.LBB1_1
+// BB_LABELS-NEXT: .byte 0
+// BB_LABELS-NEXT: .uleb128 .LBB1_2-.Lfunc_begin1
+// BB_LABELS-NEXT: .uleb128 .LBB_END1_2-.LBB1_2
+// BB_LABELS-NEXT: .byte 0
+// BB_LABELS-NEXT: .uleb128 .LBB1_3-.Lfunc_begin1
+// BB_LABELS-NEXT: .uleb128 .LBB_END1_3-.LBB1_3
+// BB_LABELS-NEXT: .byte 1
//
// BB_WORLD: .section .text.world,"ax",@progbits{{$}}
// BB_WORLD: world:
Index: clang/docs/UsersManual.rst
===================================================================
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -1700,9 +1700,11 @@
**-fbasic-block-sections=[labels, all, list=<arg>, none]**
- Controls whether Clang emits a label for each basic block. Further, with
- values "all" and "list=arg", each basic block or a subset of basic blocks
- can be placed in its own unique section.
+ Controls how Clang emits text sections for basic blocks. With values "all"
+ and "list=arg", each basic block or a subset of basic blocks can be placed in
+ its own unique section. With the "labels" value, normal sections are emitted,
+ but a ".bb_addr_map" section is emitted which would include address offsets
+ for each basic block in the program, relative to the function address.
With the ``list=<arg>`` option, a file containing the subset of basic blocks
that need to placed in unique sections can be specified. The format of the
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
