diff --git a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h --- a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h +++ b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h @@ -15,7 +15,6 @@ #ifndef LLVM_CODEGEN_BASICBLOCKSECTIONSPROFILEREADER_H #define LLVM_CODEGEN_BASICBLOCKSECTIONSPROFILEREADER_H -#include "llvm/ADT/SmallSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" @@ -26,7 +25,6 @@ #include "llvm/Support/Error.h" #include "llvm/Support/LineIterator.h" #include "llvm/Support/MemoryBuffer.h" -using namespace llvm; namespace llvm { @@ -47,7 +45,8 @@ static char ID; BasicBlockSectionsProfileReader(const MemoryBuffer *Buf) - : ImmutablePass(ID), MBuf(Buf) { + : ImmutablePass(ID), MBuf(Buf), + LineIt(*Buf, /*SkipBlanks=*/true, /*CommentMarker=*/'#') { initializeBasicBlockSectionsProfileReaderPass( *PassRegistry::getPassRegistry()); }; @@ -84,12 +83,30 @@ return R == FuncAliasMap.end() ? FuncName : R->second; } + // Returns a profile parsing error for the current line. + Error createProfileParseError(Twine Message) const { + return make_error( + Twine("invalid profile " + MBuf->getBufferIdentifier() + " at line " + + Twine(LineIt.line_number()) + ": " + Message), + inconvertibleErrorCode()); + } + // Reads the basic block sections profile for functions in this module. Error ReadProfile(); + // Reads version 0 profile. + // TODO: Remove this function once version 0 is deprecated. + Error ReadV0Profile(); + + // Reads version 1 profile. + Error ReadV1Profile(); + // This contains the basic-block-sections profile. const MemoryBuffer *MBuf = nullptr; + // Iterator to the line being parsed. + line_iterator LineIt; + // Map from every function name in the module to its debug info filename or // empty string if no debug info is available. StringMap> FunctionNameToDIFilename; diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp --- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp +++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp @@ -18,9 +18,10 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" -#include "llvm/CodeGen/Passes.h" #include "llvm/IR/DebugInfoMetadata.h" +#include "llvm/Pass.h" #include "llvm/Support/Error.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/LineIterator.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" @@ -46,34 +47,118 @@ : std::pair(false, SmallVector{}); } -// Basic Block Sections can be enabled for a subset of machine basic blocks. -// This is done by passing a file containing names of functions for which basic -// block sections are desired. Additionally, machine basic block ids of the -// functions can also be specified for a finer granularity. Moreover, a cluster -// of basic blocks could be assigned to the same section. -// Optionally, a debug-info filename can be specified for each function to allow -// distinguishing internal-linkage functions of the same name. -// A file with basic block sections for all of function main and three blocks -// for function foo (of which 1 and 2 are placed in a cluster) looks like this: -// (Profile for function foo is only loaded when its debug-info filename -// matches 'path/to/foo_file.cc'). -// ---------------------------- -// list.txt: -// !main -// !foo M=path/to/foo_file.cc -// !!1 2 -// !!4 -Error BasicBlockSectionsProfileReader::ReadProfile() { - assert(MBuf); - line_iterator LineIt(*MBuf, /*SkipBlanks=*/true, /*CommentMarker=*/'#'); +// Reads the version 1 basic block sections profile. Profile for each function +// is encoded as follows: m f +// ... c c +// ... +// Module name specifier (starting with 'm') is optional and allows +// distinguishing profile for internal-linkage functions with the same name. If +// not specified, it will apply to any function with the same name. Function +// name specifier (starting with 'f') can specify multiple function name +// aliases. Basic block clusters are specified by 'c' and specify the cluster of +// basic blocks, and the internal order in which they must be placed in the same +// section. +Error BasicBlockSectionsProfileReader::ReadV1Profile() { + auto FI = ProgramBBClusterInfo.end(); + + // Current cluster ID corresponding to this function. + unsigned CurrentCluster = 0; + // Current position in the current cluster. + unsigned CurrentPosition = 0; + + // Temporary set to ensure every basic block ID appears once in the clusters + // of a function. + SmallSet FuncBBIDs; + + // Debug-info-based module filename for the current function. Empty string + // means no filename. + StringRef DIFilename; + + for (; !LineIt.is_at_eof(); ++LineIt) { + StringRef S(*LineIt); + char Specifier = S[0]; + S = S.drop_front().trim(); + SmallVector Values; + S.split(Values, ' '); + switch (Specifier) { + case '@': + break; + case 'm': + if (Values.size() != 1) { + return createProfileParseError(Twine("invalid module name value: ") + + S); + } + DIFilename = sys::path::remove_leading_dotslash(Values[0]); + continue; + case 'f': { + bool FunctionFound = any_of(Values, [&](StringRef Alias) { + auto It = FunctionNameToDIFilename.find(Alias); + // No match if this function name is not found in this module. + if (It == FunctionNameToDIFilename.end()) + return false; + // Return a match if debug-info-filename is not specified. Otherwise, + // check for equality. + return DIFilename.empty() || It->second.equals(DIFilename); + }); + if (!FunctionFound) { + // Skip the following profile by setting the profile iterator (FI) to + // the past-the-end element. + FI = ProgramBBClusterInfo.end(); + DIFilename = ""; + continue; + } + for (size_t i = 1; i < Values.size(); ++i) + FuncAliasMap.try_emplace(Values[i], Values.front()); + + // Prepare for parsing clusters of this function name. + // Start a new cluster map for this function name. + auto R = ProgramBBClusterInfo.try_emplace(Values.front()); + // Report error when multiple profiles have been specified for the same + // function. + if (!R.second) + return createProfileParseError("duplicate profile for function '" + + Values.front() + "'."); + FI = R.first; + CurrentCluster = 0; + FuncBBIDs.clear(); + // We won't need DIFilename anymore. Clean it up to avoid its application + // on the next function. + DIFilename = ""; + continue; + } + case 'c': + // Skip the profile when we the profile iterator (FI) refers to the + // past-the-end element. + if (FI == ProgramBBClusterInfo.end()) + break; + // Reset current cluster position. + CurrentPosition = 0; + for (auto BBIDStr : Values) { + unsigned long long BBID; + if (getAsUnsignedInteger(BBIDStr, 10, BBID)) + return createProfileParseError(Twine("unsigned integer expected: '") + + BBIDStr + "'."); + if (!FuncBBIDs.insert(BBID).second) + return createProfileParseError( + Twine("duplicate basic block id found '") + BBIDStr + "'."); + if (BBID == 0 && CurrentPosition) + return createProfileParseError( + "entry BB (0) does not begin a cluster."); - auto invalidProfileError = [&](auto Message) { - return make_error( - Twine("Invalid profile " + MBuf->getBufferIdentifier() + " at line " + - Twine(LineIt.line_number()) + ": " + Message), - inconvertibleErrorCode()); - }; + FI->second.emplace_back( + BBClusterInfo{((unsigned)BBID), CurrentCluster, CurrentPosition++}); + } + CurrentCluster++; + continue; + default: + return createProfileParseError(Twine("invalid specifer: ") + + Twine(Specifier)); + } + } + return Error::success(); +} +Error BasicBlockSectionsProfileReader::ReadV0Profile() { auto FI = ProgramBBClusterInfo.end(); // Current cluster ID corresponding to this function. @@ -105,13 +190,14 @@ for (auto BBIDStr : BBIDs) { unsigned long long BBID; if (getAsUnsignedInteger(BBIDStr, 10, BBID)) - return invalidProfileError(Twine("Unsigned integer expected: '") + - BBIDStr + "'."); + return createProfileParseError(Twine("unsigned integer expected: '") + + BBIDStr + "'."); if (!FuncBBIDs.insert(BBID).second) - return invalidProfileError(Twine("Duplicate basic block id found '") + - BBIDStr + "'."); + return createProfileParseError( + Twine("duplicate basic block id found '") + BBIDStr + "'."); if (BBID == 0 && CurrentPosition) - return invalidProfileError("Entry BB (0) does not begin a cluster."); + return createProfileParseError( + "entry BB (0) does not begin a cluster."); FI->second.emplace_back( BBClusterInfo{((unsigned)BBID), CurrentCluster, CurrentPosition++}); @@ -126,10 +212,10 @@ DIFilename = sys::path::remove_leading_dotslash(DIFilenameStr.substr(2)); if (DIFilename.empty()) - return invalidProfileError("Empty module name specifier."); + return createProfileParseError("empty module name specifier."); } else if (!DIFilenameStr.empty()) { - return invalidProfileError("Unknown string found: '" + DIFilenameStr + - "'."); + return createProfileParseError("unknown string found: '" + + DIFilenameStr + "'."); } // Function aliases are separated using '/'. We use the first function // name for the cluster info mapping and delegate all other aliases to @@ -160,8 +246,8 @@ // Report error when multiple profiles have been specified for the same // function. if (!R.second) - return invalidProfileError("Duplicate profile for function '" + - Aliases.front() + "'."); + return createProfileParseError("duplicate profile for function '" + + Aliases.front() + "'."); FI = R.first; CurrentCluster = 0; FuncBBIDs.clear(); @@ -170,6 +256,50 @@ return Error::success(); } +// Basic Block Sections can be enabled for a subset of machine basic blocks. +// This is done by passing a file containing names of functions for which basic +// block sections are desired. Additionally, machine basic block ids of the +// functions can also be specified for a finer granularity. Moreover, a cluster +// of basic blocks could be assigned to the same section. +// Optionally, a debug-info filename can be specified for each function to allow +// distinguishing internal-linkage functions of the same name. +// A file with basic block sections for all of function main and three blocks +// for function foo (of which 1 and 2 are placed in a cluster) looks like this: +// (Profile for function foo is only loaded when its debug-info filename +// matches 'path/to/foo_file.cc'). +// ---------------------------- +// list.txt: +// !main +// !foo M=path/to/foo_file.cc +// !!1 2 +// !!4 +Error BasicBlockSectionsProfileReader::ReadProfile() { + assert(MBuf); + + unsigned long long Version = 0; + StringRef FirstLine(*LineIt); + if (FirstLine.consume_front("v")) { + if (getAsUnsignedInteger(FirstLine, 10, Version)) { + return createProfileParseError(Twine("version number expected: ") + + FirstLine); + } + if (Version > 1) { + return createProfileParseError(Twine("invalid profile version: ") + + Twine(Version)); + } + ++LineIt; + } + + switch (Version) { + case 0: + return ReadV0Profile(); + case 1: + return ReadV1Profile(); + default: + llvm_unreachable("Invalid profile version."); + } +} + bool BasicBlockSectionsProfileReader::doInitialization(Module &M) { if (!MBuf) return false; diff --git a/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll b/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll --- a/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll +++ b/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll @@ -3,25 +3,25 @@ ; RUN: echo '!!1 4' >> %t1 ; RUN: echo '!!1' >> %t1 ; RUN: not --crash llc < %s -O0 -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t1 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR1 -; CHECK-ERROR1: LLVM ERROR: Invalid profile {{.*}} at line 3: Duplicate basic block id found '1'. +; CHECK-ERROR1: LLVM ERROR: invalid profile {{.*}} at line 3: duplicate basic block id found '1'. ; RUN: echo '!dummy1' > %t2 ; RUN: echo '!!4 0' >> %t2 ; RUN: not --crash llc < %s -O0 -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t2 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR2 -; CHECK-ERROR2: LLVM ERROR: Invalid profile {{.*}} at line 2: Entry BB (0) does not begin a cluster. +; CHECK-ERROR2: LLVM ERROR: invalid profile {{.*}} at line 2: entry BB (0) does not begin a cluster. ; RUN: echo '!dummy1' > %t3 ; RUN: echo '!!-1' >> %t3 ; RUN: not --crash llc < %s -O0 -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t3 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR3 -; CHECK-ERROR3: LLVM ERROR: Invalid profile {{.*}} at line 2: Unsigned integer expected: '-1'. +; CHECK-ERROR3: LLVM ERROR: invalid profile {{.*}} at line 2: unsigned integer expected: '-1'. ; RUN: echo '!dummy1 /path/to/filename' > %t4 ; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t4 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR4 -; CHECK-ERROR4: LLVM ERROR: Invalid profile {{.*}} at line 1: Unknown string found: '/path/to/filename'. +; CHECK-ERROR4: LLVM ERROR: invalid profile {{.*}} at line 1: unknown string found: '/path/to/filename'. ; RUN: echo '!dummy2 M=test_dir/test_file' > %t5 ; RUN: echo '!dummy2 M=test_dir/test_file' >> %t5 ; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t5 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR5 -; CHECK-ERROR5: LLVM ERROR: Invalid profile {{.*}} at line 2: Duplicate profile for function 'dummy2'. +; CHECK-ERROR5: LLVM ERROR: invalid profile {{.*}} at line 2: duplicate profile for function 'dummy2'. ; RUN: echo '!dummy1 M=' >> %t6 ; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t6 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR6 -; CHECK-ERROR6: LLVM ERROR: Invalid profile {{.*}} at line 1: Empty module name specifier. +; CHECK-ERROR6: LLVM ERROR: invalid profile {{.*}} at line 1: empty module name specifier. define i32 @dummy1(i32 %x, i32 %y, i32 %z) { entry: diff --git a/llvm/test/CodeGen/X86/basic-block-sections-clusters.ll b/llvm/test/CodeGen/X86/basic-block-sections-clusters.ll --- a/llvm/test/CodeGen/X86/basic-block-sections-clusters.ll +++ b/llvm/test/CodeGen/X86/basic-block-sections-clusters.ll @@ -6,13 +6,23 @@ ; RUN: echo '!foo' > %t1 ; RUN: echo '!!0 2' >> %t1 ; RUN: echo '!!1' >> %t1 +; RUN: echo 'v1' > %t2 +; RUN: echo 'f foo' >> %t2 +; RUN: echo 'c 0 2' >> %t2 +; RUN: echo 'c 1' >> %t2 +; ; RUN: llc < %s -O0 -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t1 | FileCheck %s -check-prefix=LINUX-SECTIONS1 +; RUN: llc < %s -O0 -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t2 | FileCheck %s -check-prefix=LINUX-SECTIONS1 ; ; Test2: Basic blocks #1 and #3 will be placed in the same section. ; All other BBs (including the entry block) go into the function's section. -; RUN: echo '!foo' > %t2 -; RUN: echo '!!1 3' >> %t2 -; RUN: llc < %s -O0 -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t2 | FileCheck %s -check-prefix=LINUX-SECTIONS2 +; RUN: echo '!foo' > %t3 +; RUN: echo '!!1 3' >> %t3 +; RUN: echo 'v1' > %t4 +; RUN: echo 'f foo' >> %t4 +; RUN: echo 'c 1 3' >> %t4 +; RUN: llc < %s -O0 -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t3 | FileCheck %s -check-prefix=LINUX-SECTIONS2 +; RUN: llc < %s -O0 -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t4 | FileCheck %s -check-prefix=LINUX-SECTIONS2 define void @foo(i1 zeroext) nounwind { %2 = alloca i8, align 1 diff --git a/llvm/test/CodeGen/X86/basic-block-sections-cold.ll b/llvm/test/CodeGen/X86/basic-block-sections-cold.ll --- a/llvm/test/CodeGen/X86/basic-block-sections-cold.ll +++ b/llvm/test/CodeGen/X86/basic-block-sections-cold.ll @@ -1,9 +1,13 @@ ; 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: echo '!_Z3bazb' > %t1 +; RUN: echo '!!0' >> %t1 +; RUN: echo 'v1' > %t2 +; RUN: echo 'f _Z3bazb' >> %t2 +; RUN: echo 'c 0' >> %t2 +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t1 -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t2 -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t1 -unique-basic-block-section-names -bbsections-cold-text-prefix=".text.unlikely." | FileCheck %s -check-prefix=LINUX-SPLIT define void @_Z3bazb(i1 zeroext %0) nounwind { br i1 %0, label %2, label %4 diff --git a/llvm/test/CodeGen/X86/basic-block-sections-list.ll b/llvm/test/CodeGen/X86/basic-block-sections-list.ll --- a/llvm/test/CodeGen/X86/basic-block-sections-list.ll +++ b/llvm/test/CodeGen/X86/basic-block-sections-list.ll @@ -1,8 +1,13 @@ ; Check the basic block sections list option. -; RUN: echo '!_Z3foob' > %t -; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-FUNCTION-SECTION -; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-NO-FUNCTION-SECTION -; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t -unique-basic-block-section-names --bbsections-guided-section-prefix=false | FileCheck %s -check-prefix=LINUX-SECTIONS-NO-GUIDED-PREFIX +; RUN: echo '!_Z3foob' > %t1 +; RUN: echo 'v1' > %t2 +; RUN: echo 'f _Z3foob' >> %t2 +; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t1 -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-FUNCTION-SECTION +; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t1 -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-NO-FUNCTION-SECTION +; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t1 -unique-basic-block-section-names --bbsections-guided-section-prefix=false | FileCheck %s -check-prefix=LINUX-SECTIONS-NO-GUIDED-PREFIX +; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t2 -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-FUNCTION-SECTION +; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t2 -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-NO-FUNCTION-SECTION +; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t2 -unique-basic-block-section-names --bbsections-guided-section-prefix=false | FileCheck %s -check-prefix=LINUX-SECTIONS-NO-GUIDED-PREFIX define i32 @_Z3foob(i1 zeroext %0) nounwind { %2 = alloca i32, align 4 diff --git a/llvm/test/CodeGen/X86/basic-block-sections-module1.ll b/llvm/test/CodeGen/X86/basic-block-sections-module1.ll --- a/llvm/test/CodeGen/X86/basic-block-sections-module1.ll +++ b/llvm/test/CodeGen/X86/basic-block-sections-module1.ll @@ -14,6 +14,30 @@ ; RUN: echo '!test M=./path/to/dir/test_filename' > %t4 ; RUN: echo '!!0' >> %t4 ; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t4 | FileCheck %s -check-prefix=WRONG-MODULE +;; Version 1 profile. +;; Specify the right filename. +; RUN: echo 'v1' > %t5 +; RUN: echo 'm /path/to/dir/test_filename' >> %t5 +; RUN: echo 'f test' >> %t5 +; RUN: echo 'c 0' >> %t5 +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t5 | FileCheck %s -check-prefix=RIGHT-MODULE +;; Specify no filename and verify that the profile is ingested. +; RUN: echo 'v1' > %t6 +; RUN: echo 'f test' >> %t6 +; RUN: echo 'c 0' >> %t6 +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t6 | FileCheck %s -check-prefix=NO-MODULE +;; Specify wrong filenames and verify that the profile is not ingested. +; RUN: echo 'v1' > %t7 +; RUN: echo 'm test_filename' >> %t7 +; RUN: echo 'f test' >> %t7 +; RUN: echo 'c 0' >> %t7 +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t7 | FileCheck %s -check-prefix=WRONG-MODULE +; RUN: echo 'v1' > %t8 +; RUN: echo 'm ./path/to/dir/test_filename' >> %t8 +; RUN: echo 'f test' >> %t8 +; RUN: echo 'c 0' >> %t8 +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t8 | FileCheck %s -check-prefix=WRONG-MODULE + define dso_local i32 @test(i32 noundef %0) #0 !dbg !10 { %2 = alloca i32, align 4