-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[LVer][profcheck] explicitly set unknown branch weights for the versioned/unversioned selector #164507
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: users/mtrofin/10-21-_slu_profcheck_propagate_profile_for_branches_on_injected_conditions
Are you sure you want to change the base?
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
6810b9e to
63c8f11
Compare
63c8f11 to
7b55a08
Compare
|
@llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesWe don't have sufficient information to know when the versioned (or unversioned) loop variant will be taken, so we mark the branch as having "unknown" probabilities. Issue #147390 Full diff: https://github.com/llvm/llvm-project/pull/164507.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/LoopVersioning.cpp b/llvm/lib/Transforms/Utils/LoopVersioning.cpp
index ec2e6c1ab796b..4786819d18fa4 100644
--- a/llvm/lib/Transforms/Utils/LoopVersioning.cpp
+++ b/llvm/lib/Transforms/Utils/LoopVersioning.cpp
@@ -23,6 +23,7 @@
#include "llvm/IR/Dominators.h"
#include "llvm/IR/MDBuilder.h"
#include "llvm/IR/PassManager.h"
+#include "llvm/IR/ProfDataUtils.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/Cloning.h"
@@ -109,8 +110,13 @@ void LoopVersioning::versionLoop(
// Insert the conditional branch based on the result of the memchecks.
Instruction *OrigTerm = RuntimeCheckBB->getTerminator();
Builder.SetInsertPoint(OrigTerm);
- Builder.CreateCondBr(RuntimeCheck, NonVersionedLoop->getLoopPreheader(),
- VersionedLoop->getLoopPreheader());
+ auto *BI =
+ Builder.CreateCondBr(RuntimeCheck, NonVersionedLoop->getLoopPreheader(),
+ VersionedLoop->getLoopPreheader());
+ // We don't know what the probability of executing the versioned vs the
+ // unversioned variants is.
+ setExplicitlyUnknownBranchWeightsIfProfiled(
+ *BI, *BI->getParent()->getParent(), DEBUG_TYPE);
OrigTerm->eraseFromParent();
// The loops merge in the original exit block. This is now dominated by the
diff --git a/llvm/test/Transforms/LoopDistribute/basic-with-memchecks.ll b/llvm/test/Transforms/LoopDistribute/basic-with-memchecks.ll
index 97ea2c6708dad..2828882afe779 100644
--- a/llvm/test/Transforms/LoopDistribute/basic-with-memchecks.ll
+++ b/llvm/test/Transforms/LoopDistribute/basic-with-memchecks.ll
@@ -28,7 +28,7 @@ target triple = "x86_64-apple-macosx10.10.0"
@E = common global ptr null, align 8
; CHECK-LABEL: @f(
-define void @f() {
+define void @f() !prof !{!"function_entry_count", i32 10} {
entry:
%a = load ptr, ptr @A, align 8
%b = load ptr, ptr @B, align 8
@@ -55,7 +55,7 @@ entry:
; CHECK: = icmp
; CHECK-NOT: = icmp
-; CHECK: br i1 %conflict.rdx15, label %for.body.ph.lver.orig, label %for.body.ph.ldist1
+; CHECK: br i1 %conflict.rdx15, label %for.body.ph.lver.orig, label %for.body.ph.ldist1, !prof ![[PROF1:[0-9]]]
; The non-distributed loop that the memchecks fall back on.
@@ -289,3 +289,4 @@ attributes #1 = { nounwind convergent }
!0 = distinct !{!0, !1}
!1 = !{!"llvm.loop.distribute.enable", i1 true}
+; CHECK: ![[PROF1]] = !{!"unknown", !"loop-versioning"}
|
7b55a08 to
577971f
Compare
54eacf7 to
f696a5d
Compare
577971f to
e745aaf
Compare
f696a5d to
c34f582
Compare
e745aaf to
452c2f5
Compare
c34f582 to
a2080ca
Compare
ac02f48 to
9d2017c
Compare
13775b3 to
cf1d8b6
Compare
9d2017c to
19c883d
Compare
| // We don't know what the probability of executing the versioned vs the | ||
| // unversioned variants is. | ||
| setExplicitlyUnknownBranchWeightsIfProfiled( | ||
| *BI, *BI->getParent()->getParent(), DEBUG_TYPE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| *BI, *BI->getParent()->getParent(), DEBUG_TYPE); | |
| *BI, *BI->getFunction(), DEBUG_TYPE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, looks like the argument can be removed alltogether #166028
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or not, looks like InstCombine passes disconnected instructions
cf1d8b6 to
7a5b8b0
Compare
19c883d to
a5d8823
Compare
7a5b8b0 to
3778df5
Compare
a5d8823 to
df8cbd1
Compare
3778df5 to
d8f5eb6
Compare
2e48d8d to
7a7a7f2
Compare
d8f5eb6 to
4e408ee
Compare
…oned/unversioned selector
7a7a7f2 to
3cbf2c9
Compare
4e408ee to
0bf540a
Compare

We don't have sufficient information to know when the versioned (or unversioned) loop variant will be taken, so we mark the branch as having "unknown" probabilities.
Issue #147390