From c13b046de338d2751283b1abbe9dd61e96ac05c8 Mon Sep 17 00:00:00 2001 From: Rahman Lavaee Date: Tue, 27 Jun 2023 20:11:45 +0000 Subject: [PATCH] [Propeller] Match debug info filenames from profiles to distinguish internal linkage functions with the same names. Basic block sections profiles are ingested based on the function name. However, conflicts may occur when internal linkage functions with the same symbol name are linked into the binary (for instance static functions defined in different modules). Currently, these functions cannot be optimized unless we use `-funique-internal-linkage-names` (D89617) to enforce unique symbol names. However, we have found that `-funique-internal-linkage-names` does not play well with inline assembly code which refers to the symbol via its symbol name. For example, the Linux kernel does not build with this option. This patch implements a new feature which allows differentiating profiles based on the debug info filenames associated with each function. When specified, the given path is compared against the debug info filename of the matching function and profile is ingested only when the debug info filenames match. Backward-compatibility is guaranteed as omitting the specifiers from the profile would allow them to be matched by function name only. Also specifiers can be included for a subset of functions only. Reviewed By: shenhan Differential Revision: https://reviews.llvm.org/D146770 --- .../llvm/CodeGen/BasicBlockSectionsProfileReader.h | 15 ++- llvm/lib/CodeGen/BasicBlockSections.cpp | 14 ++- .../CodeGen/BasicBlockSectionsProfileReader.cpp | 83 ++++++++++++-- .../X86/basic-block-sections-clusters-error.ll | 40 +++++-- .../CodeGen/X86/basic-block-sections-module1.ll | 45 ++++++++ .../CodeGen/X86/basic-block-sections-module2.ll | 122 +++++++++++++++++++++ 6 files changed, 292 insertions(+), 27 deletions(-) create mode 100644 llvm/test/CodeGen/X86/basic-block-sections-module1.ll create mode 100644 llvm/test/CodeGen/X86/basic-block-sections-module2.ll diff --git a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h index b944c6e..cd27fea 100644 --- a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h +++ b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h @@ -16,15 +16,16 @@ #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" #include "llvm/ADT/StringRef.h" +#include "llvm/IR/Module.h" #include "llvm/InitializePasses.h" #include "llvm/Pass.h" #include "llvm/Support/Error.h" #include "llvm/Support/LineIterator.h" #include "llvm/Support/MemoryBuffer.h" - using namespace llvm; namespace llvm { @@ -73,8 +74,9 @@ public: std::pair> getBBClusterInfoForFunction(StringRef FuncName) const; - /// Read profiles of basic blocks if available here. - void initializePass() override; + // Initializes the FunctionNameToDIFilename map for the current module and + // then reads the profile for matching functions. + bool doInitialization(Module &M) override; private: StringRef getAliasName(StringRef FuncName) const { @@ -82,9 +84,16 @@ private: return R == FuncAliasMap.end() ? FuncName : R->second; } + // Reads the basic block sections profile for functions in this module. + Error ReadProfile(); + // This contains the basic-block-sections profile. const MemoryBuffer *MBuf = nullptr; + // Map from every function name in the module to its debug info filename or + // empty string if no debug info is available. + StringMap> FunctionNameToDIFilename; + // This encapsulates the BB cluster information for the whole program. // // For every function name, it contains the cluster information for (all or diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp index 5cbd53f..6967ca5 100644 --- a/llvm/lib/CodeGen/BasicBlockSections.cpp +++ b/llvm/lib/CodeGen/BasicBlockSections.cpp @@ -123,10 +123,16 @@ public: } // end anonymous namespace char BasicBlockSections::ID = 0; -INITIALIZE_PASS(BasicBlockSections, "bbsections-prepare", - "Prepares for basic block sections, by splitting functions " - "into clusters of basic blocks.", - false, false) +INITIALIZE_PASS_BEGIN( + BasicBlockSections, "bbsections-prepare", + "Prepares for basic block sections, by splitting functions " + "into clusters of basic blocks.", + false, false) +INITIALIZE_PASS_DEPENDENCY(BasicBlockSectionsProfileReader) +INITIALIZE_PASS_END(BasicBlockSections, "bbsections-prepare", + "Prepares for basic block sections, by splitting functions " + "into clusters of basic blocks.", + false, false) // This function updates and optimizes the branching instructions of every basic // block in a given function to account for changes in the layout. diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp index 8929955..5dede45 100644 --- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp +++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp @@ -14,12 +14,17 @@ #include "llvm/CodeGen/BasicBlockSectionsProfileReader.h" #include "llvm/ADT/SmallSet.h" +#include "llvm/ADT/SmallString.h" #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/Support/Error.h" #include "llvm/Support/LineIterator.h" #include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/Path.h" +#include using namespace llvm; @@ -46,17 +51,19 @@ BasicBlockSectionsProfileReader::getBBClusterInfoForFunction( // 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 +// !foo M=path/to/foo_file.cc // !!1 2 // !!4 -static Error getBBClusterInfo(const MemoryBuffer *MBuf, - ProgramBBClusterInfoMapTy &ProgramBBClusterInfo, - StringMap &FuncAliasMap) { +Error BasicBlockSectionsProfileReader::ReadProfile() { assert(MBuf); line_iterator LineIt(*MBuf, /*SkipBlanks=*/true, /*CommentMarker=*/'#'); @@ -87,9 +94,10 @@ static Error getBBClusterInfo(const MemoryBuffer *MBuf, break; // Check for second "!" which indicates a cluster of basic blocks. if (S.consume_front("!")) { + // Skip the profile when we the profile iterator (FI) refers to the + // past-the-end element. if (FI == ProgramBBClusterInfo.end()) - return invalidProfileError( - "Cluster list does not follow a function name specifier."); + continue; SmallVector BBIDs; S.split(BBIDs, ' '); // Reset current cluster position. @@ -109,18 +117,52 @@ static Error getBBClusterInfo(const MemoryBuffer *MBuf, BBClusterInfo{((unsigned)BBID), CurrentCluster, CurrentPosition++}); } CurrentCluster++; - } else { // This is a function name specifier. + } else { + // This is a function name specifier. It may include a debug info filename + // specifier starting with `M=`. + auto [AliasesStr, DIFilenameStr] = S.split(' '); + SmallString<128> DIFilename; + if (DIFilenameStr.startswith("M=")) { + DIFilename = + sys::path::remove_leading_dotslash(DIFilenameStr.substr(2)); + if (DIFilename.empty()) + return invalidProfileError("Empty module name specifier."); + } else if (!DIFilenameStr.empty()) { + return invalidProfileError("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 // this one. SmallVector Aliases; - S.split(Aliases, '/'); + AliasesStr.split(Aliases, '/'); + bool FunctionFound = any_of(Aliases, [&](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(); + continue; + } for (size_t i = 1; i < Aliases.size(); ++i) FuncAliasMap.try_emplace(Aliases[i], Aliases.front()); // Prepare for parsing clusters of this function name. // Start a new cluster map for this function name. - FI = ProgramBBClusterInfo.try_emplace(Aliases.front()).first; + auto R = ProgramBBClusterInfo.try_emplace(Aliases.front()); + // Report error when multiple profiles have been specified for the same + // function. + if (!R.second) + return invalidProfileError("Duplicate profile for function '" + + Aliases.front() + "'."); + FI = R.first; CurrentCluster = 0; FuncBBIDs.clear(); } @@ -128,11 +170,28 @@ static Error getBBClusterInfo(const MemoryBuffer *MBuf, return Error::success(); } -void BasicBlockSectionsProfileReader::initializePass() { +bool BasicBlockSectionsProfileReader::doInitialization(Module &M) { if (!MBuf) - return; - if (auto Err = getBBClusterInfo(MBuf, ProgramBBClusterInfo, FuncAliasMap)) + return false; + // Get the function name to debug info filename mapping. + FunctionNameToDIFilename.clear(); + for (const Function &F : M) { + SmallString<128> DIFilename; + if (F.isDeclaration()) + continue; + DISubprogram *Subprogram = F.getSubprogram(); + if (Subprogram) { + llvm::DICompileUnit *CU = Subprogram->getUnit(); + if (CU) + DIFilename = sys::path::remove_leading_dotslash(CU->getFilename()); + } + [[maybe_unused]] bool inserted = + FunctionNameToDIFilename.try_emplace(F.getName(), DIFilename).second; + assert(inserted); + } + if (auto Err = ReadProfile()) report_fatal_error(std::move(Err)); + return false; } ImmutablePass * diff --git a/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll b/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll index 28c79e2..15705b8 100644 --- a/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll +++ b/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll @@ -1,25 +1,49 @@ ; BB cluster sections error handling -; RUN: echo '!f' > %t1 +; RUN: echo '!dummy1' > %t1 ; 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'. -; RUN: echo '!f' > %t2 +; 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. -; RUN: echo '!f' > %t3 +; 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'. -; RUN: echo '!!1' > %t4 -; RUN: echo '!f' >> %t4 -; RUN: not --crash llc < %s -O0 -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t4 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR4 -; CHECK-ERROR4: LLVM ERROR: Invalid profile {{.*}} at line 1: Cluster list does not follow a function name specifier. +; 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'. +; 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'. +; 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. -define i32 @dummy(i32 %x, i32 %y, i32 %z) { +define i32 @dummy1(i32 %x, i32 %y, i32 %z) { entry: %tmp = mul i32 %x, %y %tmp2 = add i32 %tmp, %z ret i32 %tmp2 } + +define i32 @dummy2(i32 %x, i32 %y, i32 %z) !dbg !4 { + entry: + %tmp = mul i32 %x, %y + %tmp2 = add i32 %tmp, %z + ret i32 %tmp2 +} + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!2, !3} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1) +!1 = !DIFile(filename: "test_dir/test_file", directory: "test_dir") +!2 = !{i32 7, !"Dwarf Version", i32 5} +!3 = !{i32 2, !"Debug Info Version", i32 3} +!4 = distinct !DISubprogram(name: "dummy1", scope: !1, unit: !0) + + diff --git a/llvm/test/CodeGen/X86/basic-block-sections-module1.ll b/llvm/test/CodeGen/X86/basic-block-sections-module1.ll new file mode 100644 index 0000000..1f11c4f --- /dev/null +++ b/llvm/test/CodeGen/X86/basic-block-sections-module1.ll @@ -0,0 +1,45 @@ +;; This file tests specifying the debug info filename in the basic block sections profile. +;; Specify the right filename. +; RUN: echo '!test M=/path/to/dir/test_filename' > %t1 +; RUN: echo '!!0' >> %t1 +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t1 | FileCheck %s -check-prefix=RIGHT-MODULE +;; Specify no filename and verify that the profile is ingested. +; RUN: echo '!test' > %t2 +; RUN: echo '!!0' >> %t2 +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t2 | FileCheck %s -check-prefix=NO-MODULE +;; Specify wrong filenames and verify that the profile is not ingested. +; RUN: echo '!test M=test_filename' > %t3 +; RUN: echo '!!0' >> %t3 +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t3 | FileCheck %s -check-prefix=WRONG-MODULE +; 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 + +define dso_local i32 @test(i32 noundef %0) #0 !dbg !10 { + %2 = alloca i32, align 4 + %3 = alloca i32, align 4 + store i32 %0, i32* %3, align 4 + %4 = load i32, i32* %3, align 4 + %5 = icmp slt i32 %4, 0 + br i1 %5, label %6, label %7 +6: ; preds = %1 + store i32 -1, i32* %2, align 4 + ret i32 0 +7: + ret i32 1 +} + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!2, !3} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1) +!1 = !DIFile(filename: "/path/to/dir/test_filename", directory: "/path/to/dir") +!2 = !{i32 7, !"Dwarf Version", i32 5} +!3 = !{i32 2, !"Debug Info Version", i32 3} +!10 = distinct !DISubprogram(name: "test", scope: !1, unit: !0) + +;; Check that the split section is created when using the correct module name, or no module name. +; +; RIGHT-MODULE: .section .text.split.test,"ax",@progbits +; NO-MODULE: .section .text.split.test,"ax",@progbits +; WRONG-MODULE-NOT: .section .text.split.test,"ax",@progbits diff --git a/llvm/test/CodeGen/X86/basic-block-sections-module2.ll b/llvm/test/CodeGen/X86/basic-block-sections-module2.ll new file mode 100644 index 0000000..6964fce --- /dev/null +++ b/llvm/test/CodeGen/X86/basic-block-sections-module2.ll @@ -0,0 +1,122 @@ +;; This file tests various ways of specifying the debug info filename in the basic block sections profile. +;; Specifying correct filenames. +; RUN: echo '!test1 M=./test_dirname1/test_filename1' > %t1 +; RUN: echo '!!0' >> %t1 +; RUN: echo '!test2 M=.//test_filename2' >> %t1 +; RUN: echo '!!0' >> %t1 +; RUN: echo '!test3 M=test_filename3' >> %t1 +; RUN: echo '!!0' >> %t1 +; RUN: echo '!test4 M=/test_dirname4/test_filename4' >> %t1 +; RUN: echo '!!0' >> %t1 +; RUN: echo '!test5' >> %t1 +; RUN: echo '!!0' >> %t1 +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t1 | FileCheck %s -check-prefix=RIGHT-MODULE +;; Specifying wrong filenames. +; RUN: echo '!test1 M=/test_dirname/test_filename1' > %t2 +; RUN: echo '!!0' >> %t2 +; RUN: echo '!test1 M=../test_filename1' >> %t2 +; RUN: echo '!!0' >> %t2 +; RUN: echo '!test2 M=.test_filename2' >> %t2 +; RUN: echo '!!0' >> %t2 +; RUN: echo '!test4 M=./test_dirname4/test_filename4' >> %t2 +; RUN: echo '!!0' >> %t2 +; RUN: echo '!test5 M=any_filename' >> %t1 +; RUN: echo '!!0' >> %t1 +; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=%t2 | FileCheck %s -check-prefix=WRONG-MODULE + +define dso_local i32 @test1(i32 noundef %0) #0 !dbg !10 { + %2 = alloca i32, align 4 + %3 = alloca i32, align 4 + store i32 %0, i32* %3, align 4 + %4 = load i32, i32* %3, align 4 + %5 = icmp slt i32 %4, 0 + br i1 %5, label %6, label %7 +6: ; preds = %1 + store i32 -1, i32* %2, align 4 + ret i32 0 +7: + ret i32 1 +} + +define dso_local i32 @test2(i32 noundef %0) #0 !dbg !11 { + %2 = alloca i32, align 4 + %3 = alloca i32, align 4 + store i32 %0, i32* %3, align 4 + %4 = load i32, i32* %3, align 4 + %5 = icmp slt i32 %4, 0 + br i1 %5, label %6, label %7 +6: ; preds = %1 + store i32 -1, i32* %2, align 4 + ret i32 0 +7: + ret i32 1 +} + +define dso_local i32 @test3(i32 noundef %0) #0 !dbg !12 { + %2 = alloca i32, align 4 + %3 = alloca i32, align 4 + store i32 %0, i32* %3, align 4 + %4 = load i32, i32* %3, align 4 + %5 = icmp slt i32 %4, 0 + br i1 %5, label %6, label %7 +6: ; preds = %1 + store i32 -1, i32* %2, align 4 + ret i32 0 +7: + ret i32 1 +} + +define dso_local i32 @test4(i32 noundef %0) #0 !dbg !13 { + %2 = alloca i32, align 4 + %3 = alloca i32, align 4 + store i32 %0, i32* %3, align 4 + %4 = load i32, i32* %3, align 4 + %5 = icmp slt i32 %4, 0 + br i1 %5, label %6, label %7 +6: ; preds = %1 + store i32 -1, i32* %2, align 4 + ret i32 0 +7: + ret i32 1 +} + +define dso_local i32 @test5(i32 noundef %0) #0 { + %2 = alloca i32, align 4 + %3 = alloca i32, align 4 + store i32 %0, i32* %3, align 4 + %4 = load i32, i32* %3, align 4 + %5 = icmp slt i32 %4, 0 + br i1 %5, label %6, label %7 +6: ; preds = %1 + store i32 -1, i32* %2, align 4 + ret i32 0 +7: + ret i32 1 +} + +!llvm.dbg.cu = !{!0, !1, !2, !3} +!llvm.module.flags = !{!8, !9} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !4) +!1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !5) +!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !6) +!3 = distinct !DICompileUnit(language: DW_LANG_C99, file: !7) +!4 = !DIFile(filename: "test_dirname1/test_filename1", directory: "test_dirname1") +!5 = !DIFile(filename: "test_filename2", directory: "") +!6 = !DIFile(filename: "./test_filename3", directory: ".") +!7 = !DIFile(filename: "/test_dirname4/test_filename4", directory: "/test_dirname4") +!8 = !{i32 7, !"Dwarf Version", i32 5} +!9 = !{i32 2, !"Debug Info Version", i32 3} +!10 = distinct !DISubprogram(name: "test1", scope: !4, unit: !0) +!11 = distinct !DISubprogram(name: "test2", scope: !5, unit: !1) +!12 = distinct !DISubprogram(name: "test3", scope: !6, unit: !2) +!13 = distinct !DISubprogram(name: "test4", scope: !7, unit: !3) + +;; Check that the split section is created when using the correct module name, or no module name. +; +; RIGHT-MODULE: .section .text.split.test1,"ax",@progbits +; RIGHT-MODULE: .section .text.split.test2,"ax",@progbits +; RIGHT-MODULE: .section .text.split.test3,"ax",@progbits +; RIGHT-MODULE: .section .text.split.test4,"ax",@progbits +; RIGHT-MODULE: .section .text.split.test5,"ax",@progbits +; WRONG-MODULE-NOT: .section .text.split.test{{[1-5]+}},"ax",@progbits -- 2.7.4