[llvm] 5ac48ef - [Propeller] Use a bit-field struct for the metdata fields of BBEntry.
Rahman Lavaee via llvm-commits
llvm-commits at lists.llvm.org
Thu May 11 11:21:46 PDT 2023
Author: Rahman Lavaee
Date: 2023-05-11T11:21:26-07:00
New Revision: 5ac48ef51393e6d8392182ad439a22002571d554
URL: https://github.com/llvm/llvm-project/commit/5ac48ef51393e6d8392182ad439a22002571d554
DIFF: https://github.com/llvm/llvm-project/commit/5ac48ef51393e6d8392182ad439a22002571d554.diff
LOG: [Propeller] Use a bit-field struct for the metdata fields of BBEntry.
This patch encapsulates the encoding and decoding logic of basic block metadata into the Metadata struct, and also reduces the decoded size of `SHT_LLVM_BB_ADDR_MAP` section.
The patch would've looked more readable if we could use designated initializer, but that is a c++20 feature.
Reviewed By: jhenderson
Differential Revision: https://reviews.llvm.org/D148360
Added:
Modified:
llvm/include/llvm/Object/ELFTypes.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/Object/ELF.cpp
llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test
llvm/unittests/Object/ELFObjectFileTest.cpp
llvm/unittests/Object/ELFTypesTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index 608b15b48ad6c..5b1a0b32fbb5e 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -799,32 +799,57 @@ struct BBAddrMap {
uint64_t Addr; // Function address
// Struct representing the BBAddrMap information for one basic block.
struct BBEntry {
+ struct Metadata {
+ bool HasReturn : 1; // If this block ends with a return (or tail call).
+ bool HasTailCall : 1; // If this block ends with a tail call.
+ bool IsEHPad : 1; // If this is an exception handling block.
+ bool CanFallThrough : 1; // If this block can fall through to its next.
+
+ bool operator==(const Metadata &Other) const {
+ return HasReturn == Other.HasReturn &&
+ HasTailCall == Other.HasTailCall && IsEHPad == Other.IsEHPad &&
+ CanFallThrough == Other.CanFallThrough;
+ }
+
+ // Encodes this struct as a uint32_t value.
+ uint32_t encode() const {
+ return static_cast<uint32_t>(HasReturn) |
+ (static_cast<uint32_t>(HasTailCall) << 1) |
+ (static_cast<uint32_t>(IsEHPad) << 2) |
+ (static_cast<uint32_t>(CanFallThrough) << 3);
+ }
+
+ // Decodes and returns a Metadata struct from a uint32_t value.
+ static Expected<Metadata> decode(uint32_t V) {
+ Metadata MD{/*HasReturn=*/static_cast<bool>(V & 1),
+ /*HasTailCall=*/static_cast<bool>(V & (1 << 1)),
+ /*IsEHPad=*/static_cast<bool>(V & (1 << 2)),
+ /*CanFallThrough=*/static_cast<bool>(V & (1 << 3))};
+ if (MD.encode() != V)
+ return createStringError(
+ std::error_code(), "invalid encoding for BBEntry::Metadata: 0x%x",
+ V);
+ return MD;
+ }
+ };
+
uint32_t ID; // Unique ID of this basic block.
uint32_t Offset; // Offset of basic block relative to function start.
uint32_t Size; // Size of the basic block.
+ Metadata MD; // Metdata for this basic block.
- // The following fields are decoded from the Metadata field. The encoding
- // happens in AsmPrinter.cpp:getBBAddrMapMetadata.
- bool HasReturn; // If this block ends with a return (or tail call).
- bool HasTailCall; // If this block ends with a tail call.
- bool IsEHPad; // If this is an exception handling block.
- bool CanFallThrough; // If this block can fall through to its next.
-
- BBEntry(uint32_t ID, uint32_t Offset, uint32_t Size, uint32_t Metadata)
- : ID(ID), Offset(Offset), Size(Size), HasReturn(Metadata & 1),
- HasTailCall(Metadata & (1 << 1)), IsEHPad(Metadata & (1 << 2)),
- CanFallThrough(Metadata & (1 << 3)){};
+ BBEntry(uint32_t ID, uint32_t Offset, uint32_t Size, Metadata MD)
+ : ID(ID), Offset(Offset), Size(Size), MD(MD){};
bool operator==(const BBEntry &Other) const {
return ID == Other.ID && Offset == Other.Offset && Size == Other.Size &&
- HasReturn == Other.HasReturn && HasTailCall == Other.HasTailCall &&
- IsEHPad == Other.IsEHPad && CanFallThrough == Other.CanFallThrough;
+ MD == Other.MD;
}
- bool hasReturn() const { return HasReturn; }
- bool hasTailCall() const { return HasTailCall; }
- bool isEHPad() const { return IsEHPad; }
- bool canFallThrough() const { return CanFallThrough; }
+ bool hasReturn() const { return MD.HasReturn; }
+ bool hasTailCall() const { return MD.HasTailCall; }
+ bool isEHPad() const { return MD.IsEHPad; }
+ bool canFallThrough() const { return MD.CanFallThrough; }
};
std::vector<BBEntry> BBEntries; // Basic block entries for this function.
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 9420da90dca32..746b7e409cf75 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -99,6 +99,7 @@
#include "llvm/MC/MCTargetOptions.h"
#include "llvm/MC/MCValue.h"
#include "llvm/MC/SectionKind.h"
+#include "llvm/Object/ELFTypes.h"
#include "llvm/Pass.h"
#include "llvm/Remarks/RemarkStreamer.h"
#include "llvm/Support/Casting.h"
@@ -1330,21 +1331,15 @@ void AsmPrinter::emitFrameAlloc(const MachineInstr &MI) {
MCConstantExpr::create(FrameOffset, OutContext));
}
-/// Returns the BB metadata to be emitted in the .llvm_bb_addr_map section for a
-/// given basic block. This can be used to capture more precise profile
-/// information. We use the last 4 bits (LSBs) to encode the following
-/// information:
-/// * (1): set if return block (ret or tail call).
-/// * (2): set if ends with a tail call.
-/// * (3): set if exception handling (EH) landing pad.
-/// * (4): set if the block can fall through to its next.
-/// The remaining bits are zero.
-static unsigned getBBAddrMapMetadata(const MachineBasicBlock &MBB) {
+/// Returns the BB metadata to be emitted in the SHT_LLVM_BB_ADDR_MAP section
+/// for a given basic block. This can be used to capture more precise profile
+/// information.
+static uint32_t getBBAddrMapMetadata(const MachineBasicBlock &MBB) {
const TargetInstrInfo *TII = MBB.getParent()->getSubtarget().getInstrInfo();
- return ((unsigned)MBB.isReturnBlock()) |
- ((!MBB.empty() && TII->isTailCall(MBB.back())) << 1) |
- (MBB.isEHPad() << 2) |
- (const_cast<MachineBasicBlock &>(MBB).canFallThrough() << 3);
+ return object::BBAddrMap::BBEntry::Metadata{
+ MBB.isReturnBlock(), !MBB.empty() && TII->isTailCall(MBB.back()),
+ MBB.isEHPad(), const_cast<MachineBasicBlock &>(MBB).canFallThrough()}
+ .encode();
}
void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
diff --git a/llvm/lib/Object/ELF.cpp b/llvm/lib/Object/ELF.cpp
index b9e73848b171a..e4f42f01acdff 100644
--- a/llvm/lib/Object/ELF.cpp
+++ b/llvm/lib/Object/ELF.cpp
@@ -674,6 +674,7 @@ ELFFile<ELFT>::decodeBBAddrMap(const Elf_Shdr &Sec,
DataExtractor::Cursor Cur(0);
Error ULEBSizeErr = Error::success();
+ Error MetadataDecodeErr = Error::success();
// Helper to extract and decode the next ULEB128 value as uint32_t.
// Returns zero and sets ULEBSizeErr if the ULEB128 value exceeds the uint32_t
// limit.
@@ -694,7 +695,8 @@ ELFFile<ELFT>::decodeBBAddrMap(const Elf_Shdr &Sec,
};
uint8_t Version = 0;
- while (!ULEBSizeErr && Cur && Cur.tell() < Content.size()) {
+ while (!ULEBSizeErr && !MetadataDecodeErr && Cur &&
+ Cur.tell() < Content.size()) {
if (Sec.sh_type == ELF::SHT_LLVM_BB_ADDR_MAP) {
Version = Data.getU8(Cur);
if (!Cur)
@@ -722,24 +724,32 @@ ELFFile<ELFT>::decodeBBAddrMap(const Elf_Shdr &Sec,
std::vector<BBAddrMap::BBEntry> BBEntries;
uint32_t PrevBBEndOffset = 0;
for (uint32_t BlockIndex = 0;
- !ULEBSizeErr && Cur && (BlockIndex < NumBlocks); ++BlockIndex) {
+ !MetadataDecodeErr && !ULEBSizeErr && Cur && (BlockIndex < NumBlocks);
+ ++BlockIndex) {
uint32_t ID = Version >= 2 ? ReadULEB128AsUInt32() : BlockIndex;
uint32_t Offset = ReadULEB128AsUInt32();
uint32_t Size = ReadULEB128AsUInt32();
- uint32_t Metadata = ReadULEB128AsUInt32();
+ uint32_t MD = ReadULEB128AsUInt32();
if (Version >= 1) {
// Offset is calculated relative to the end of the previous BB.
Offset += PrevBBEndOffset;
PrevBBEndOffset = Offset + Size;
}
- BBEntries.push_back({ID, Offset, Size, Metadata});
+ Expected<BBAddrMap::BBEntry::Metadata> MetadataOrErr =
+ BBAddrMap::BBEntry::Metadata::decode(MD);
+ if (!MetadataOrErr) {
+ MetadataDecodeErr = MetadataOrErr.takeError();
+ break;
+ }
+ BBEntries.push_back({ID, Offset, Size, *MetadataOrErr});
}
FunctionEntries.push_back({Address, std::move(BBEntries)});
}
- // Either Cur is in the error state, or ULEBSizeError is set (not both), but
- // we join the two errors here to be safe.
- if (!Cur || ULEBSizeErr)
- return joinErrors(Cur.takeError(), std::move(ULEBSizeErr));
+ // Either Cur is in the error state, or we have an error in ULEBSizeErr or
+ // MetadataDecodeErr (but not both), but we join all errors here to be safe.
+ if (!Cur || ULEBSizeErr || MetadataDecodeErr)
+ return joinErrors(joinErrors(Cur.takeError(), std::move(ULEBSizeErr)),
+ std::move(MetadataDecodeErr));
return FunctionEntries;
}
diff --git a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test
index a40cf0d1a0624..d84d0ef4c2b21 100644
--- a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test
+++ b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test
@@ -21,6 +21,10 @@
# RUN: yaml2obj --docnum=1 %s -DBITS=32 -DSIZE=6 -o %t2.o
# RUN: llvm-readobj %t2.o --bb-addr-map 2>&1 | FileCheck %s -DOFFSET=0x00000006 -DFILE=%t2.o --check-prefix=TRUNCATED
+## Check that invalid metadata can be handled.
+# RUN: yaml2obj --docnum=1 %s -DBITS=32 -DMETADATA=0xF000002 -o %t3.o
+# RUN: llvm-readobj %t3.o --bb-addr-map 2>&1 | FileCheck %s -DFILE=%t3.o --check-prefix=INVALIDMD
+
# CHECK: BBAddrMap [
# CHECK-NEXT: Function {
# CHECK-NEXT: At: [[ADDR]]
@@ -97,6 +101,9 @@
# TRUNCATED-NEXT: }
# TRUNCATED-NEXT: ]
+# INVALIDMD: BBAddrMap [
+# INVALIDMD-NEXT: warning: '[[FILE]]': unable to dump SHT_LLVM_BB_ADDR_MAP section with index 3: invalid encoding for BBEntry::Metadata: 0xf000002
+
--- !ELF
FileHeader:
Class: ELFCLASS[[BITS]]
@@ -120,7 +127,7 @@ Sections:
- ID: 0
AddressOffset: 0x0
Size: 0x1
- Metadata: 0xF0000002
+ Metadata: [[METADATA=0x2]]
- ID: 2
AddressOffset: 0x3
Size: 0x4
diff --git a/llvm/unittests/Object/ELFObjectFileTest.cpp b/llvm/unittests/Object/ELFObjectFileTest.cpp
index 74d6d72cc8bae..724fce7ca8827 100644
--- a/llvm/unittests/Object/ELFObjectFileTest.cpp
+++ b/llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -661,10 +661,10 @@ TEST(ELFObjectFileTest, ReadBBAddrMap) {
Metadata: 0x8
)");
- BBAddrMap E1 = {0x11111, {{1, 0x0, 0x1, 0x2}}};
- BBAddrMap E2 = {0x22222, {{2, 0x0, 0x2, 0x4}}};
- BBAddrMap E3 = {0x33333, {{0, 0x0, 0x3, 0x6}}};
- BBAddrMap E4 = {0x44444, {{0, 0x0, 0x4, 0x8}}};
+ BBAddrMap E1 = {0x11111, {{1, 0x0, 0x1, {false, true, false, false}}}};
+ BBAddrMap E2 = {0x22222, {{2, 0x0, 0x2, {false, false, true, false}}}};
+ BBAddrMap E3 = {0x33333, {{0, 0x0, 0x3, {false, true, true, false}}}};
+ BBAddrMap E4 = {0x44444, {{0, 0x0, 0x4, {false, false, false, true}}}};
std::vector<BBAddrMap> Section0BBAddrMaps = {E4};
std::vector<BBAddrMap> Section1BBAddrMaps = {E3};
diff --git a/llvm/unittests/Object/ELFTypesTest.cpp b/llvm/unittests/Object/ELFTypesTest.cpp
index aae71ef385a72..c4d822c55fd26 100644
--- a/llvm/unittests/Object/ELFTypesTest.cpp
+++ b/llvm/unittests/Object/ELFTypesTest.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
#include "llvm/Object/ELFTypes.h"
+#include "llvm/Testing/Support/Error.h"
#include "gtest/gtest.h"
#include <iostream>
@@ -61,3 +62,34 @@ TEST(ELFTypesTest, NoteTest) {
TestData.getElfNote("AMD", ELF::NT_AMDGPU_METADATA, ArrayRef<uint8_t>());
EXPECT_EQ(Note3.getDescAsStringRef(4), StringRef(""));
}
+
+TEST(ELFTypesTest, BBEntryMetadataEncodingTest) {
+ const std::array<BBAddrMap::BBEntry::Metadata, 6> Decoded = {
+ {{false, false, false, false},
+ {true, false, false, false},
+ {false, true, false, false},
+ {false, false, true, false},
+ {false, false, false, true},
+ {true, true, true, true}}};
+ const std::array<uint32_t, 6> Encoded = {{0, 1, 2, 4, 8, 15}};
+ for (size_t i = 0; i < Decoded.size(); ++i)
+ EXPECT_EQ(Decoded[i].encode(), Encoded[i]);
+ for (size_t i = 0; i < Encoded.size(); ++i) {
+ Expected<BBAddrMap::BBEntry::Metadata> MetadataOrError =
+ BBAddrMap::BBEntry::Metadata::decode(Encoded[i]);
+ ASSERT_THAT_EXPECTED(MetadataOrError, Succeeded());
+ EXPECT_EQ(*MetadataOrError, Decoded[i]);
+ }
+}
+
+TEST(ELFTypesTest, BBEntryMetadataInvalidEncodingTest) {
+ const std::array<std::string, 2> Errors = {
+ "invalid encoding for BBEntry::Metadata: 0xffff",
+ "invalid encoding for BBEntry::Metadata: 0x100001"};
+ const std::array<uint32_t, 2> Values = {{0xFFFF, 0x100001}};
+ for (size_t i = 0; i < Values.size(); ++i) {
+ EXPECT_THAT_ERROR(
+ BBAddrMap::BBEntry::Metadata::decode(Values[i]).takeError(),
+ FailedWithMessage(Errors[i]));
+ }
+}
More information about the llvm-commits
mailing list