[PATCH] D95593: Detect Source Drift with Propeller's basic-block-sections=list=

[Sriraman Tallam via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc32f3998029d: Detect Source Drift with Propeller. (authored by tmsriram).

Changed prior to commit:
  https://reviews.llvm.org/D95593?vs=320024&id=320260#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95593/new/

https://reviews.llvm.org/D95593

Files:
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/test/CodeGen/X86/basic-block-sections-source-drift.ll


Index: llvm/test/CodeGen/X86/basic-block-sections-source-drift.ll
===================================================================
--- /dev/null
+++ 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
Index: llvm/lib/CodeGen/BasicBlockSections.cpp
===================================================================
--- llvm/lib/CodeGen/BasicBlockSections.cpp
+++ 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<bool> 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<MDTuple>(Existing);
+    for (auto &N : Tuple->operands())
+      if (cast<MDString>(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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D95593.320260.patch
Type: text/x-patch
Size: 3503 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210130/cdf5f5bf/attachment.bin>