Conversation
35c4f40 to
549da06
Compare
rlavaee
left a comment
There was a problem hiding this comment.
Please add a test for this. Just to make sure that the hashes are read and errors can be detected (like basic-block-sections-clusters-error.ll).
| @@ -287,6 +296,25 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() { | |||
| } | |||
| continue; | |||
| } | |||
| case 'h': { // Basic block hash secifier. | |||
| // Skip the profile when we the profile iterator (FI) refers to the | |||
| if (getAsUnsignedInteger(BBIDStr, 10, BBID)) | ||
| return createProfileParseError(Twine("unsigned integer expected: '") + | ||
| BBIDStr + "'"); | ||
| HashStr.consume_front("0x"); |
There was a problem hiding this comment.
Maybe we don't need to print 0x at all and assume that the hash is always hexadecimal.
| @@ -493,6 +521,12 @@ uint64_t BasicBlockSectionsProfileReaderWrapperPass::getEdgeCount( | |||
| return BBSPR.getEdgeCount(FuncName, SrcBBID, SinkBBID); | |||
| } | |||
|
|
|||
| std::pair<bool, FunctionPathAndClusterInfo> | |||
| BasicBlockSectionsProfileReaderWrapperPass::getFunctionPathAndClusterInfo( | |||
There was a problem hiding this comment.
I don't see this being used in this PR.
There was a problem hiding this comment.
This method will be used in the next PR, and we have removed it from this PR.
549da06 to
f5ae6cd
Compare
|
@llvm/pr-subscribers-backend-x86 Author: None (wdx727) ChangesAdding Matching and Inference Functionality to Propeller. For detailed information, please refer to the following RFC: https://discourse.llvm.org/t/rfc-adding-matching-and-inference-functionality-to-propeller/86238. co-authors: lifengxiang1025 [lifengxiang@kuaishou.com](mailto:lifengxiang@kuaishou.com); zcfh [wuminghui03@kuaishou.com](mailto:wuminghui03@kuaishou.com) Full diff: https://github.com/llvm/llvm-project/pull/164223.diff 4 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
index 48650a6df22ff..4114c02c39f06 100644
--- a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
+++ b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
@@ -54,6 +54,8 @@ struct FunctionPathAndClusterInfo {
DenseMap<UniqueBBID, uint64_t> NodeCounts;
// Edge counts for each edge, stored as a nested map.
DenseMap<UniqueBBID, DenseMap<UniqueBBID, uint64_t>> EdgeCounts;
+ // Hash for each basic block.
+ DenseMap<unsigned, uint64_t> BBHashes;
};
class BasicBlockSectionsProfileReader {
diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
index fbcd614b85d18..67b56de904e91 100644
--- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
@@ -287,6 +287,24 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
}
continue;
}
+ case 'h': { // Basic block hash secifier.
+ // Skip the profile when the profile iterator (FI) refers to the
+ // past-the-end element.
+ if (FI == ProgramPathAndClusterInfo.end())
+ continue;
+ for (auto BBIDHashStr : Values) {
+ auto [BBIDStr, HashStr] = BBIDHashStr.split(':');
+ unsigned long long BBID = 0, Hash = 0;
+ if (getAsUnsignedInteger(BBIDStr, 10, BBID))
+ return createProfileParseError(Twine("unsigned integer expected: '") +
+ BBIDStr + "'");
+ if (getAsUnsignedInteger(HashStr, 16, Hash))
+ return createProfileParseError(Twine("unsigned integer expected: '") +
+ HashStr + "'");
+ FI->second.BBHashes[BBID] = Hash;
+ }
+ continue;
+ }
default:
return createProfileParseError(Twine("invalid specifier: '") +
Twine(Specifier) + "'");
diff --git a/llvm/test/CodeGen/X86/basic-block-sections-bb-hash.ll b/llvm/test/CodeGen/X86/basic-block-sections-bb-hash.ll
new file mode 100644
index 0000000000000..0c249d7c0618f
--- /dev/null
+++ b/llvm/test/CodeGen/X86/basic-block-sections-bb-hash.ll
@@ -0,0 +1,48 @@
+; BB section test with basic block hashes.
+;
+; RUN: llc %s -O0 -mtriple=x86_64-pc-linux -function-sections -filetype=obj -basic-block-address-map -emit-bb-hash -o %t.o
+; RUN: obj2yaml %t.o -o %t.yaml
+;
+;; Profile for version 1:
+; RUN: echo 'v1' > %t
+; RUN: echo 'f foo' >> %t
+; RUN: echo 'g 0:10,1:9,2:1 1:8,3:8 2:2,3:2 3:11' >> %t
+; RUN: echo 'c 0 2 3' >> %t
+; RUN: grep -E '^\s+(- ID:|Hash:)' %t.yaml | \
+; RUN: grep -B1 'Hash:' | \
+; RUN: sed 's/^\s*//; s/^- ID: *//; s/Hash: *0x//' | \
+; RUN: paste -d: - - | \
+; RUN: tr '\n' ' ' | \
+; RUN: sed 's/ $/\n/; s/^/h /' >> %t
+;
+; RUN: llc < %s -O0 -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t | FileCheck %s
+;
+define void @foo(i1 zeroext) nounwind {
+ %2 = alloca i8, align 1
+ %3 = zext i1 %0 to i8
+ store i8 %3, ptr %2, align 1
+ %4 = load i8, ptr %2, align 1
+ %5 = trunc i8 %4 to i1
+ br i1 %5, label %6, label %8
+
+6: ; preds = %1
+ %7 = call i32 @bar()
+ br label %10
+
+8: ; preds = %1
+ %9 = call i32 @baz()
+ br label %10
+
+10: ; preds = %8, %6
+ ret void
+}
+
+declare i32 @bar() #1
+
+declare i32 @baz() #1
+
+; CHECK: .section .text.foo,"ax",@progbits
+; CHECK: callq baz
+; CHECK: retq
+; CHECK: .section .text.split.foo,"ax",@progbits
+; CHECK: callq bar
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 751ab76722c07..dadc94e0566a7 100644
--- a/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll
+++ b/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll
@@ -69,7 +69,20 @@
; RUN: echo 'g 0:4,1:2:3' >> %t15
; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t15 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR15
; CHECK-ERROR15: LLVM ERROR: invalid profile {{.*}} at line 4: unsigned integer expected: '2:3'
-
+; RUN: echo 'v1' > %t16
+; RUN: echo 'f dummy1' >> %t16
+; RUN: echo 'c 0 1' >> %t16
+; RUN: echo 'g 0:4,1:2' >> %t16
+; RUN: echo 'h a:1111111111111111 1:ffffffffffffffff' >> %t16
+; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t16 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR16
+; CHECK-ERROR16: LLVM ERROR: invalid profile {{.*}} at line 5: unsigned integer expected: 'a'
+; RUN: echo 'v1' > %t17
+; RUN: echo 'f dummy1' >> %t17
+; RUN: echo 'c 0 1' >> %t17
+; RUN: echo 'g 0:4,1:2' >> %t17
+; RUN: echo 'h 0:111111111111111g 1:ffffffffffffffff' >> %t17
+; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t17 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR17
+; CHECK-ERROR17: LLVM ERROR: invalid profile {{.*}} at line 5: unsigned integer expected: '111111111111111g'
define i32 @dummy1(i32 %x, i32 %y, i32 %z) {
entry:
|
3892abf to
1f62e8e
Compare
Done. |
1f62e8e to
d1db0ae
Compare
| DenseMap<UniqueBBID, uint64_t> NodeCounts; | ||
| // Edge counts for each edge, stored as a nested map. | ||
| DenseMap<UniqueBBID, DenseMap<UniqueBBID, uint64_t>> EdgeCounts; | ||
| // Hash for each basic block. |
There was a problem hiding this comment.
Please update the comment to clarify that hashes are stored for every original block (not cloned blocks), hence the map key being unsigned instead of UniqueBBID.
| return createProfileParseError(Twine("unsigned integer expected: '") + | ||
| BBIDStr + "'"); | ||
| if (getAsUnsignedInteger(HashStr, 16, Hash)) | ||
| return createProfileParseError(Twine("unsigned integer expected: '") + |
There was a problem hiding this comment.
Maybe say "unsigned integer expected in hex format" here.
| ; RUN: echo 'f foo' >> %t | ||
| ; RUN: echo 'g 0:10,1:9,2:1 1:8,3:8 2:2,3:2 3:11' >> %t | ||
| ; RUN: echo 'c 0 2 3' >> %t | ||
| ; RUN: grep -E '^\s+(- ID:|Hash:)' %t.yaml | \ |
There was a problem hiding this comment.
Could you please explain these commands with a comment? Looks like it's reading the SHT_LLVM_BB_ADDR_MAP for hashes and then putting them into the basic blocks sections profile.
There was a problem hiding this comment.
You are right. We have added the comment.
d1db0ae to
aadce86
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
…sic block hashes from propeller profile. Co-authored-by: lifengxiang1025 <lifengxiang@kuaishou.com> Co-authored-by: zcfh <wuminghui03@kuaishou.com>
aadce86 to
c990315
Compare
|
Let me know if/when you want me to merge this upstream. |
Thank you. I need you to help merge this PR. |
|
FYI, I simplified the test here to satisfy the buildbots: 4afb0e6f |
Thanks. |
…sic block hashes from propeller profile. (llvm#164223) Adding Matching and Inference Functionality to Propeller. For detailed information, please refer to the following RFC: https://discourse.llvm.org/t/rfc-adding-matching-and-inference-functionality-to-propeller/86238. This is the third PR, which is used to read basic block hashes from the propeller profile. The associated PRs are: PR1: llvm#160706 PR2: llvm#162963 co-authors: lifengxiang1025 [lifengxiang@kuaishou.com](mailto:lifengxiang@kuaishou.com); zcfh [wuminghui03@kuaishou.com](mailto:wuminghui03@kuaishou.com) Co-authored-by: lifengxiang1025 <lifengxiang@kuaishou.com> Co-authored-by: zcfh <wuminghui03@kuaishou.com>
…sic block hashes from propeller profile. (llvm#164223) Adding Matching and Inference Functionality to Propeller. For detailed information, please refer to the following RFC: https://discourse.llvm.org/t/rfc-adding-matching-and-inference-functionality-to-propeller/86238. This is the third PR, which is used to read basic block hashes from the propeller profile. The associated PRs are: PR1: llvm#160706 PR2: llvm#162963 co-authors: lifengxiang1025 [lifengxiang@kuaishou.com](mailto:lifengxiang@kuaishou.com); zcfh [wuminghui03@kuaishou.com](mailto:wuminghui03@kuaishou.com) Co-authored-by: lifengxiang1025 <lifengxiang@kuaishou.com> Co-authored-by: zcfh <wuminghui03@kuaishou.com>
…nt matching and inference and create clusters (#165868) Adding Matching and Inference Functionality to Propeller. For detailed information, please refer to the following RFC: https://discourse.llvm.org/t/rfc-adding-matching-and-inference-functionality-to-propeller/86238. This is the fourth PR, which is used to implement matching and inference and create the clusters. The associated PRs are: PR1: #160706 PR2: #162963 PR3: #164223 co-authors: lifengxiang1025 [lifengxiang@kuaishou.com](mailto:lifengxiang@kuaishou.com); zcfh [wuminghui03@kuaishou.com](mailto:wuminghui03@kuaishou.com) Co-authored-by: lifengxiang1025 <lifengxiang@kuaishou.com> Co-authored-by: zcfh <wuminghui03@kuaishou.com>
…R4: Implement matching and inference and create clusters (#165868) Adding Matching and Inference Functionality to Propeller. For detailed information, please refer to the following RFC: https://discourse.llvm.org/t/rfc-adding-matching-and-inference-functionality-to-propeller/86238. This is the fourth PR, which is used to implement matching and inference and create the clusters. The associated PRs are: PR1: llvm/llvm-project#160706 PR2: llvm/llvm-project#162963 PR3: llvm/llvm-project#164223 co-authors: lifengxiang1025 [lifengxiang@kuaishou.com](mailto:lifengxiang@kuaishou.com); zcfh [wuminghui03@kuaishou.com](mailto:wuminghui03@kuaishou.com) Co-authored-by: lifengxiang1025 <lifengxiang@kuaishou.com> Co-authored-by: zcfh <wuminghui03@kuaishou.com>
Adding Matching and Inference Functionality to Propeller. For detailed information, please refer to the following RFC: https://discourse.llvm.org/t/rfc-adding-matching-and-inference-functionality-to-propeller/86238.
This is the third PR, which is used to read basic block hashes from the propeller profile. The associated PRs are:
PR1: #160706
PR2: #162963
co-authors: lifengxiang1025 lifengxiang@kuaishou.com; zcfh wuminghui03@kuaishou.com