[llvm] [BasicBlockSections] Propose a staleness detection by checking the func cfg node num (PR #133244)
Jinjie Huang via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 27 20:01:40 PDT 2025
https://github.com/Jinjie-Huang updated https://github.com/llvm/llvm-project/pull/133244
>From 11a78d9204f6aa0c82b85c93207f725d5c530678 Mon Sep 17 00:00:00 2001
From: huangjinjie <huangjinjie at bytedance.com>
Date: Thu, 27 Mar 2025 20:54:11 +0800
Subject: [PATCH 1/3] propose a check for func cfg node num
---
.../CodeGen/BasicBlockSectionsProfileReader.h | 7 ++
llvm/lib/CodeGen/BasicBlockSections.cpp | 12 ++
.../BasicBlockSectionsProfileReader.cpp | 22 ++++
.../X86/bb-sections-cfg-node-num-check.ll | 116 ++++++++++++++++++
4 files changed, 157 insertions(+)
create mode 100644 llvm/test/CodeGen/X86/bb-sections-cfg-node-num-check.ll
diff --git a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
index 08e6a0e3ef629..51000efd7f219 100644
--- a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
+++ b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
@@ -50,6 +50,8 @@ struct FunctionPathAndClusterInfo {
// the edge a -> b (a is not cloned). The index of the path in this vector
// determines the `UniqueBBID::CloneID` of the cloned blocks in that path.
SmallVector<SmallVector<unsigned>> ClonePaths;
+ // Node count of the func CFG, record for staleness check
+ unsigned NodeCount;
};
// Provides DenseMapInfo for UniqueBBID.
@@ -94,6 +96,9 @@ class BasicBlockSectionsProfileReader {
std::pair<bool, SmallVector<BBClusterInfo>>
getClusterInfoForFunction(StringRef FuncName) const;
+ // Returns the cfg node count of the CFG for the given function.
+ unsigned getCfgNodeNumForFunction(StringRef FuncName) const;
+
// Returns the path clonings for the given function.
SmallVector<SmallVector<unsigned>>
getClonePathsForFunction(StringRef FuncName) const;
@@ -201,6 +206,8 @@ class BasicBlockSectionsProfileReaderWrapperPass : public ImmutablePass {
std::pair<bool, SmallVector<BBClusterInfo>>
getClusterInfoForFunction(StringRef FuncName) const;
+ unsigned getCfgNodeNumForFunction(StringRef FuncName) const;
+
SmallVector<SmallVector<unsigned>>
getClonePathsForFunction(StringRef FuncName) const;
diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp
index 1eedfc4b25912..ad2e86ea16c58 100644
--- a/llvm/lib/CodeGen/BasicBlockSections.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -80,6 +80,7 @@
#include "llvm/CodeGen/TargetInstrInfo.h"
#include "llvm/InitializePasses.h"
#include "llvm/Target/TargetMachine.h"
+#include "llvm/Support/WithColor.h"
#include <optional>
using namespace llvm;
@@ -311,6 +312,7 @@ bool BasicBlockSections::handleBBSections(MachineFunction &MF) {
// original layout positions and finding the original fallthroughs.
MF.RenumberBlocks();
+ unsigned NodeCount = 0;
DenseMap<UniqueBBID, BBClusterInfo> FuncClusterInfo;
if (BBSectionsType == BasicBlockSection::List) {
auto [HasProfile, ClusterInfo] =
@@ -318,6 +320,16 @@ bool BasicBlockSections::handleBBSections(MachineFunction &MF) {
.getClusterInfoForFunction(MF.getName());
if (!HasProfile)
return false;
+
+ NodeCount = getAnalysis<BasicBlockSectionsProfileReaderWrapperPass>()
+ .getCfgNodeNumForFunction(MF.getName());
+ if ((NodeCount != 0) && (NodeCount != MF.size())) {
+ WithColor::warning() << "MF " << MF.getName() << ": node count mismatch "
+ << "(profile=" << NodeCount
+ << " actual=" << MF.size() << ")\n";
+ return false;
+ }
+
for (auto &BBClusterInfo : ClusterInfo) {
FuncClusterInfo.try_emplace(BBClusterInfo.BBID, BBClusterInfo);
}
diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
index fa54640265162..06e5034522bd4 100644
--- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
@@ -69,6 +69,13 @@ BasicBlockSectionsProfileReader::getClusterInfoForFunction(
: std::pair(false, SmallVector<BBClusterInfo>());
}
+unsigned BasicBlockSectionsProfileReader::getCfgNodeNumForFunction(
+ StringRef FuncName) const {
+ auto R = ProgramPathAndClusterInfo.find(getAliasName(FuncName));
+ return R != ProgramPathAndClusterInfo.end()
+ ? R->second.NodeCount : 0;
+}
+
SmallVector<SmallVector<unsigned>>
BasicBlockSectionsProfileReader::getClonePathsForFunction(
StringRef FuncName) const {
@@ -263,6 +270,16 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
StringRef S(*LineIt);
if (S[0] == '@')
continue;
+
+ // Record the function cfg node num for staleness check
+ if (S.consume_front("$node_count")) {
+ unsigned long long NodeCount;
+ if (getAsUnsignedInteger(S.trim(), 10, NodeCount))
+ return invalidProfileError("Invalid node count value.");
+ if (FI != ProgramBBClusterInfo.end())
+ FI->second.NodeCount = (unsigned)NodeCount;
+ continue;
+ }
// Check for the leading "!"
if (!S.consume_front("!") || S.empty())
break;
@@ -433,6 +450,11 @@ BasicBlockSectionsProfileReaderWrapperPass::getClusterInfoForFunction(
return BBSPR.getClusterInfoForFunction(FuncName);
}
+unsigned BasicBlockSectionsProfileReaderWrapperPass::getCfgNodeNumForFunction(
+ StringRef FuncName) const {
+ return BBSPR.getCfgNodeNumForFunction(FuncName);
+}
+
SmallVector<SmallVector<unsigned>>
BasicBlockSectionsProfileReaderWrapperPass::getClonePathsForFunction(
StringRef FuncName) const {
diff --git a/llvm/test/CodeGen/X86/bb-sections-cfg-node-num-check.ll b/llvm/test/CodeGen/X86/bb-sections-cfg-node-num-check.ll
new file mode 100644
index 0000000000000..8d128d7f60b89
--- /dev/null
+++ b/llvm/test/CodeGen/X86/bb-sections-cfg-node-num-check.ll
@@ -0,0 +1,116 @@
+; BB cluster func cfg node num tests for v0.
+;
+;; Profile for version 0 check node num:
+; RUN: echo '!foo' > %t1
+; RUN: echo '#wrong node_count given' >> %t1
+; RUN: echo '$node_count 8' >> %t1
+; RUN: echo '!!0 5 6 18 3 4 8 9 11 15 16 12 13' >> %t1
+; RUN: echo '!bar' >> %t1
+; RUN: echo '#correct node_count given' >> %t1
+; RUN: echo '$node_count 3' >> %t1
+; RUN: echo '!!0 1 2' >> %t1
+; RUN: echo '!main' >> %t1
+
+; RUN: llc --basic-block-sections=%t1 -o %t %s 2>&1 | FileCheck %s --check-prefix=CHECK-WARNING
+; CHECK-WARNING: warning: MF foo: node count mismatch (profile=8 actual=7)
+
+define dso_local void @bar() #0 {
+ %1 = alloca i32, align 4
+ %2 = alloca i32, align 4
+ store volatile i32 0, i32* %1, align 4
+ store i32 0, i32* %2, align 4
+ br label %3
+
+; <label>:3: ; preds = %10, %0
+ %4 = load i32, i32* %2, align 4
+ %5 = icmp slt i32 %4, 1000
+ br i1 %5, label %6, label %13
+
+; <label>:6: ; preds = %3
+ %7 = load i32, i32* %2, align 4
+ %8 = load volatile i32, i32* %1, align 4
+ %9 = add nsw i32 %8, %7
+ store volatile i32 %9, i32* %1, align 4
+ br label %10
+
+; <label>:10: ; preds = %6
+ %11 = load i32, i32* %2, align 4
+ %12 = add nsw i32 %11, 1
+ store i32 %12, i32* %2, align 4
+ br label %3
+
+; <label>:13: ; preds = %3
+ ret void
+}
+
+define dso_local void @foo(i32) #0 {
+ %2 = alloca i32, align 4
+ %3 = alloca i32, align 4
+ %4 = alloca i32, align 4
+ %5 = alloca i32, align 4
+ store i32 %0, i32* %2, align 4
+ store i32 2147483640, i32* %3, align 4
+ store i32 0, i32* %4, align 4
+ br label %6
+
+; <label>:6: ; preds = %30, %1
+ %7 = load i32, i32* %4, align 4
+ %8 = load i32, i32* %2, align 4
+ %9 = icmp slt i32 %7, %8
+ br i1 %9, label %10, label %33
+
+; <label>:10: ; preds = %6
+ %11 = load i32, i32* %3, align 4
+ %12 = srem i32 %11, 100
+ store i32 %12, i32* %5, align 4
+ %13 = load i32, i32* %5, align 4
+ %14 = icmp slt i32 %13, 90
+ br i1 %14, label %15, label %21
+
+; <label>:15: ; preds = %10
+ %16 = load i32, i32* %5, align 4
+ %17 = icmp slt i32 %16, 70
+ br i1 %17, label %18, label %19
+
+; <label>:18: ; preds = %15
+ call void @bar()
+ br label %20
+
+; <label>:19: ; preds = %15
+ call void @bar()
+ br label %20
+
+; <label>:20: ; preds = %19, %18
+ br label %27
+
+; <label>:21: ; preds = %10
+ %22 = load i32, i32* %5, align 4
+ %23 = icmp slt i32 %22, 93
+ br i1 %23, label %24, label %25
+
+; <label>:24: ; preds = %21
+ call void @bar()
+ br label %26
+
+; <label>:25: ; preds = %21
+ call void @bar()
+ br label %26
+
+; <label>:26: ; preds = %25, %24
+ br label %27
+
+; <label>:27: ; preds = %26, %20
+ %28 = load i32, i32* %3, align 4
+ %29 = add nsw i32 %28, -1
+ store i32 %29, i32* %3, align 4
+ br label %30
+
+; <label>:30: ; preds = %27
+ %31 = load i32, i32* %4, align 4
+ %32 = add nsw i32 %31, 1
+ store i32 %32, i32* %4, align 4
+ br label %6
+
+; <label>:33: ; preds = %6
+ ret void
+}
>From 0c4526472189a433bbd23a84c3244b2a91b52fbb Mon Sep 17 00:00:00 2001
From: huangjinjie <huangjinjie at bytedance.com>
Date: Fri, 28 Mar 2025 10:20:53 +0800
Subject: [PATCH 2/3] propose a check for func cfg node num
---
llvm/lib/CodeGen/BasicBlockSections.cpp | 8 ++++----
llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp | 3 +--
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp
index ad2e86ea16c58..4c237fe665070 100644
--- a/llvm/lib/CodeGen/BasicBlockSections.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -79,8 +79,8 @@
#include "llvm/CodeGen/Passes.h"
#include "llvm/CodeGen/TargetInstrInfo.h"
#include "llvm/InitializePasses.h"
-#include "llvm/Target/TargetMachine.h"
#include "llvm/Support/WithColor.h"
+#include "llvm/Target/TargetMachine.h"
#include <optional>
using namespace llvm;
@@ -320,13 +320,13 @@ bool BasicBlockSections::handleBBSections(MachineFunction &MF) {
.getClusterInfoForFunction(MF.getName());
if (!HasProfile)
return false;
-
+
NodeCount = getAnalysis<BasicBlockSectionsProfileReaderWrapperPass>()
.getCfgNodeNumForFunction(MF.getName());
if ((NodeCount != 0) && (NodeCount != MF.size())) {
WithColor::warning() << "MF " << MF.getName() << ": node count mismatch "
- << "(profile=" << NodeCount
- << " actual=" << MF.size() << ")\n";
+ << "(profile=" << NodeCount
+ << " actual=" << MF.size() << ")\n";
return false;
}
diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
index 06e5034522bd4..1a41b88b9f01d 100644
--- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
@@ -72,8 +72,7 @@ BasicBlockSectionsProfileReader::getClusterInfoForFunction(
unsigned BasicBlockSectionsProfileReader::getCfgNodeNumForFunction(
StringRef FuncName) const {
auto R = ProgramPathAndClusterInfo.find(getAliasName(FuncName));
- return R != ProgramPathAndClusterInfo.end()
- ? R->second.NodeCount : 0;
+ return R != ProgramPathAndClusterInfo.end() ? R->second.NodeCount : 0;
}
SmallVector<SmallVector<unsigned>>
>From 50b284a6aca460d1794bcbdae13efeebd2536d80 Mon Sep 17 00:00:00 2001
From: huangjinjie <huangjinjie at bytedance.com>
Date: Fri, 28 Mar 2025 11:01:20 +0800
Subject: [PATCH 3/3] propose a check for func cfg node num
---
llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
index 1a41b88b9f01d..23e5d953a6c34 100644
--- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
@@ -274,8 +274,8 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
if (S.consume_front("$node_count")) {
unsigned long long NodeCount;
if (getAsUnsignedInteger(S.trim(), 10, NodeCount))
- return invalidProfileError("Invalid node count value.");
- if (FI != ProgramBBClusterInfo.end())
+ return createProfileParseError("Invalid node count value.");
+ if (FI != ProgramPathAndClusterInfo.end())
FI->second.NodeCount = (unsigned)NodeCount;
continue;
}
More information about the llvm-commits
mailing list