diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp --- a/llvm/lib/CodeGen/BasicBlockSections.cpp +++ b/llvm/lib/CodeGen/BasicBlockSections.cpp @@ -88,6 +88,12 @@ cl::desc("The text prefix to use for cold basic block clusters"), cl::init(".text.split."), cl::Hidden); +cl::opt BBSectionsDetectSourceDrift( + "bbsections-detect-source-drift", + cl::desc("This checks if there is a fdo instr. profile hash " + "mismatch for this function"), + cl::init(true), cl::Hidden); + namespace { // This struct represents the cluster information for a machine basic block. @@ -313,10 +319,42 @@ return true; } +// This checks if the source of this function has drifted since this binary was +// profiled previously. For now, we are piggy backing on what PGO does to +// detect this with instrumented profiles. PGO emits an hash of the IR and +// checks if the hash has changed. Advanced basic block layout is usually done +// on top of PGO optimized binaries and hence this check works well in practice. +static bool hasInstrProfHashMismatch(MachineFunction &MF) { + if (!BBSectionsDetectSourceDrift) + return false; + + const char MetadataName[] = "instr_prof_hash_mismatch"; + auto *Existing = MF.getFunction().getMetadata(LLVMContext::MD_annotation); + if (Existing) { + MDTuple *Tuple = cast(Existing); + for (auto &N : Tuple->operands()) + if (cast(N.get())->getString() == MetadataName) + return true; + } + + return false; +} + bool BasicBlockSections::runOnMachineFunction(MachineFunction &MF) { auto BBSectionsType = MF.getTarget().getBBSectionsType(); assert(BBSectionsType != BasicBlockSection::None && "BB Sections not enabled!"); + + // Check for source drift. If the source has changed since the profiles + // were obtained, optimizing basic blocks might be sub-optimal. + // This only applies to BasicBlockSection::List as it creates + // clusters of basic blocks using basic block ids. Source drift can + // invalidate these groupings leading to sub-optimal code generation with + // regards to performance. + if (BBSectionsType == BasicBlockSection::List && + hasInstrProfHashMismatch(MF)) + return true; + // Renumber blocks before sorting them for basic block sections. This is // useful during sorting, basic blocks in the same section will retain the // default order. This renumbering should also be done for basic block diff --git a/llvm/test/CodeGen/X86/basic-block-sections-source-drift.ll b/llvm/test/CodeGen/X86/basic-block-sections-source-drift.ll new file mode 100644 --- /dev/null +++ b/llvm/test/CodeGen/X86/basic-block-sections-source-drift.ll @@ -0,0 +1,19 @@ +; RUN: echo "!foo" > %t.order.txt +; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t.order.txt | FileCheck --check-prefix=SOURCE-DRIFT %s +; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t.order.txt -bbsections-detect-source-drift=false | FileCheck --check-prefix=HASH-CHECK-DISABLED %s + +define dso_local i32 @foo(i1 zeroext %0, i1 zeroext %1) !annotation !1 { + br i1 %0, label %5, label %3 + +3: ; preds = %2 + %4 = select i1 %1, i32 2, i32 0 + ret i32 %4 + +5: ; preds = %2 + ret i32 1 +} + +!1 = !{!"instr_prof_hash_mismatch"} + +; SOURCE-DRIFT-NOT: .section .text +; HASH-CHECK-DISABLED: .section .text