AntonBikineev created this revision.
AntonBikineev added a reviewer: hans.
Herald added a subscriber: wenlei.
Herald added a project: All.
AntonBikineev requested review of this revision.
Herald added a project: clang.
Generally, with PGO enabled the C++20 likelyhood attributes shall be
dropped assuming the profile has a good coverage. However, currently
this is not the case for the following code:
if (always_false()) [[likely]] {
...
}
The patch fixes this and drops the attribute, if the parent context was
executed in the profile. The patch still preserves the attribute, if the
parent context was not executed, e.g. to support the cases when the
profile has insufficient coverage and the programmer's assumption is
correct.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134456
Files:
clang/lib/CodeGen/CGStmt.cpp
clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
clang/test/Profile/cxx-never-executed-branch.cpp
Index: clang/test/Profile/cxx-never-executed-branch.cpp
===================================================================
--- /dev/null
+++ clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test the clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o
%t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -O2 -o - -emit-llvm
-fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+ int rando = rand();
+ int x = 0;
+ if (rando == 0) [[likely]]
+ x = 2;
+ else
+ x = 3;
+ return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+ int rando = rand();
+ int x = 0;
+ if (rando == 0) [[likely]]
+ x = 2;
+ else
+ x = 3;
+ return x;
+}
Index: clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
===================================================================
--- /dev/null
+++ clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x000000029f493458
+2
+20000
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev
+0x000000029f493458
+2
+# The function was never executed
+0
+0
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -815,11 +815,21 @@
// Prefer the PGO based weights over the likelihood attribute.
// When the build isn't optimized the metadata isn't used, so don't generate
// it.
+ // Also, differentiate between disabled PGO and a never executed branch with
+ // PGO. Assuming PGO is in use:
+ // - we want to ignore the [[likely]] attribute if the branch is never
+ // executed,
+ // - assuming the profile is poor, preserving the attribute may still be
+ // beneficial.
+ // As an approximation, preserve the attribute only if both the branch and
the
+ // parent context were not executed.
Stmt::Likelihood LH = Stmt::LH_None;
- uint64_t Count = getProfileCount(S.getThen());
- if (!Count && CGM.getCodeGenOpts().OptimizationLevel)
+ uint64_t ThenCount = getProfileCount(S.getThen());
+ if (!ThenCount && !getCurrentProfileCount() &&
+ CGM.getCodeGenOpts().OptimizationLevel) {
LH = Stmt::getLikelihood(S.getThen(), S.getElse());
- EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, Count, LH);
+ }
+ EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, ThenCount, LH);
// Emit the 'then' code.
EmitBlock(ThenBlock);
Index: clang/test/Profile/cxx-never-executed-branch.cpp
===================================================================
--- /dev/null
+++ clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test the clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o %t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+ int rando = rand();
+ int x = 0;
+ if (rando == 0) [[likely]]
+ x = 2;
+ else
+ x = 3;
+ return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+ int rando = rand();
+ int x = 0;
+ if (rando == 0) [[likely]]
+ x = 2;
+ else
+ x = 3;
+ return x;
+}
Index: clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
===================================================================
--- /dev/null
+++ clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x000000029f493458
+2
+20000
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev
+0x000000029f493458
+2
+# The function was never executed
+0
+0
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -815,11 +815,21 @@
// Prefer the PGO based weights over the likelihood attribute.
// When the build isn't optimized the metadata isn't used, so don't generate
// it.
+ // Also, differentiate between disabled PGO and a never executed branch with
+ // PGO. Assuming PGO is in use:
+ // - we want to ignore the [[likely]] attribute if the branch is never
+ // executed,
+ // - assuming the profile is poor, preserving the attribute may still be
+ // beneficial.
+ // As an approximation, preserve the attribute only if both the branch and the
+ // parent context were not executed.
Stmt::Likelihood LH = Stmt::LH_None;
- uint64_t Count = getProfileCount(S.getThen());
- if (!Count && CGM.getCodeGenOpts().OptimizationLevel)
+ uint64_t ThenCount = getProfileCount(S.getThen());
+ if (!ThenCount && !getCurrentProfileCount() &&
+ CGM.getCodeGenOpts().OptimizationLevel) {
LH = Stmt::getLikelihood(S.getThen(), S.getElse());
- EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, Count, LH);
+ }
+ EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, ThenCount, LH);
// Emit the 'then' code.
EmitBlock(ThenBlock);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
