Skip to content

Conversation

@llvmbot
Copy link
Member

@llvmbot llvmbot commented Aug 31, 2024

Backport 46a4132

Requested by: @tchaikov

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Aug 31, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Aug 31, 2024

@mtrofin What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Aug 31, 2024

@llvm/pr-subscribers-coroutines
@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-llvm-transforms

Author: None (llvmbot)

Changes

Backport 46a4132

Requested by: @tchaikov


Full diff: https://github.com/llvm/llvm-project/pull/106823.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp (+10-4)
  • (added) llvm/test/Transforms/Coroutines/coro-pgo-setbranchweights.ll (+42)
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 1ce8f58c1aa140..4924d5a317478f 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -1625,11 +1625,17 @@ void PGOUseFunc::setBranchWeights() {
       continue;
 
     // We have a non-zero Branch BB.
-    unsigned Size = BBCountInfo.OutEdges.size();
-    SmallVector<uint64_t, 2> EdgeCounts(Size, 0);
+
+    // SuccessorCount can be greater than OutEdgesCount, because
+    // removed edges don't appear in OutEdges.
+    unsigned OutEdgesCount = BBCountInfo.OutEdges.size();
+    unsigned SuccessorCount = BB.getTerminator()->getNumSuccessors();
+    assert(OutEdgesCount <= SuccessorCount);
+
+    SmallVector<uint64_t, 2> EdgeCounts(SuccessorCount, 0);
     uint64_t MaxCount = 0;
-    for (unsigned s = 0; s < Size; s++) {
-      const PGOUseEdge *E = BBCountInfo.OutEdges[s];
+    for (unsigned It = 0; It < OutEdgesCount; It++) {
+      const PGOUseEdge *E = BBCountInfo.OutEdges[It];
       const BasicBlock *SrcBB = E->SrcBB;
       const BasicBlock *DestBB = E->DestBB;
       if (DestBB == nullptr)
diff --git a/llvm/test/Transforms/Coroutines/coro-pgo-setbranchweights.ll b/llvm/test/Transforms/Coroutines/coro-pgo-setbranchweights.ll
new file mode 100644
index 00000000000000..4f5f936606ca3f
--- /dev/null
+++ b/llvm/test/Transforms/Coroutines/coro-pgo-setbranchweights.ll
@@ -0,0 +1,42 @@
+; RUN: rm -rf %t && split-file %s %t
+
+; RUN: llvm-profdata merge %t/a.proftext -o %t/a.profdata
+; RUN: opt < %t/a.ll --passes=pgo-instr-use -pgo-test-profile-file=%t/a.profdata
+
+;--- a.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-redhat-linux-gnu"
+
+define void @_bar() presplitcoroutine personality ptr null {
+  %1 = call token @llvm.coro.save(ptr null)
+  %2 = call i8 @llvm.coro.suspend(token none, i1 false)
+  switch i8 %2, label %5 [
+    i8 0, label %3
+    i8 1, label %4
+  ]
+
+3:                                                ; preds = %0
+  ret void
+
+4:                                                ; preds = %0
+  ret void
+
+5:                                                ; preds = %0
+  ret void
+}
+
+declare token @llvm.coro.save(ptr)
+
+declare i8 @llvm.coro.suspend(token, i1)
+
+;--- a.proftext
+# IR level Instrumentation Flag
+:ir
+
+_bar
+# Func Hash:
+1063705160175073211
+# Num Counters:
+2
+1
+0

@tchaikov
Copy link
Contributor

@mtrofin thank you for approving this. could you help merge this change? as i don't have the write permission of this repo.

@boomanaiden154
Copy link
Contributor

This needs to be merged by one of the release managers (the release branches are protected). Tobias will probably get to it by early next week.

@tchaikov
Copy link
Contributor

@boomanaiden154 thank you for the pointer. i see now.

@tru tru merged commit f131edf into llvm:release/19.x Sep 1, 2024
@github-actions
Copy link

github-actions bot commented Sep 1, 2024

@tchaikov (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coroutines C++20 coroutines llvm:transforms PGO Profile Guided Optimizations

Projects

Development

Successfully merging this pull request may close these issues.

6 participants