[PATCH] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

[Anton Bikineev via Phabricator via cfe-commits]

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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D134456.462221.patch
Type: text/x-patch
Size: 2706 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220922/fa3f3470/attachment.bin>