[profiling] Fix profile counter increment when emitting selects (PR32019)

Clang has logic to lower certain conditional expressions directly into
llvm select instructions. However, it does not emit the correct profile
counter increment as it does this: it emits an unconditional increment
of the counter for the 'then branch', even if the value selected is from
the 'else branch' (this is PR32019).

That means, given the following snippet, we would report that "0" is
selected twice, and that "1" is never selected:

  int f1(int x) {
    return x ? 0 : 1;
               ^2  ^0
  }

  f1(0);
  f1(1);

Fix the problem by using the instrprof_increment_step intrinsic to do
the proper increment.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@296231 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp
index 529642cb83e6daf3db230a82bba1926949ea8ce5..43267833821e7137547dbacfb69f3e8b0735c0eb 100644 (file)
--- a/lib/CodeGen/CGExprScalar.cpp
+++ b/lib/CodeGen/CGExprScalar.cpp
@@ -3414,9 +3414,11 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
   // safe to evaluate the LHS and RHS unconditionally.
   if (isCheapEnoughToEvaluateUnconditionally(lhsExpr, CGF) &&
       isCheapEnoughToEvaluateUnconditionally(rhsExpr, CGF)) {
-    CGF.incrementProfileCounter(E);
-
     llvm::Value *CondV = CGF.EvaluateExprAsBool(condExpr);
+    llvm::Value *StepV = Builder.CreateZExtOrBitCast(CondV, CGF.Int64Ty);
+
+    CGF.incrementProfileCounter(E, StepV);
+
     llvm::Value *LHS = Visit(lhsExpr);
     llvm::Value *RHS = Visit(rhsExpr);
     if (!LHS) {

diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index 72105802b45a9407a4c85ea5a60cc5b59f2f3965..e011c1167fd31a9505ba449d5823a904cbe00ed8 100644 (file)
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -1127,10 +1127,11 @@ private:
                                             uint64_t LoopCount);
 
 public:
-  /// Increment the profiler's counter for the given statement.
-  void incrementProfileCounter(const Stmt *S) {
+  /// Increment the profiler's counter for the given statement by \p StepV.
+  /// If \p StepV is null, the default increment is 1.
+  void incrementProfileCounter(const Stmt *S, llvm::Value *StepV = nullptr) {
     if (CGM.getCodeGenOpts().hasProfileClangInstr())
-      PGO.emitCounterIncrement(Builder, S);
+      PGO.emitCounterIncrement(Builder, S, StepV);
     PGO.setCurrentStmt(S);
   }
 

diff --git a/lib/CodeGen/CodeGenPGO.cpp b/lib/CodeGen/CodeGenPGO.cpp
index 530ee9b4e9e266a1aa587f6fc8e74c66833d0bb8..2c805b05a61acc7f6a5b2b037eb3e8e44827c9dc 100644 (file)
--- a/lib/CodeGen/CodeGenPGO.cpp
+++ b/lib/CodeGen/CodeGenPGO.cpp
@@ -739,7 +739,8 @@ CodeGenPGO::applyFunctionAttributes(llvm::IndexedInstrProfReader *PGOReader,
   Fn->setEntryCount(FunctionCount);
 }
 
-void CodeGenPGO::emitCounterIncrement(CGBuilderTy &Builder, const Stmt *S) {
+void CodeGenPGO::emitCounterIncrement(CGBuilderTy &Builder, const Stmt *S,
+                                      llvm::Value *StepV) {
   if (!CGM.getCodeGenOpts().hasProfileClangInstr() || !RegionCounterMap)
     return;
   if (!Builder.GetInsertBlock())
@@ -747,11 +748,17 @@ void CodeGenPGO::emitCounterIncrement(CGBuilderTy &Builder, const Stmt *S) {
 
   unsigned Counter = (*RegionCounterMap)[S];
   auto *I8PtrTy = llvm::Type::getInt8PtrTy(CGM.getLLVMContext());
-  Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::instrprof_increment),
-                     {llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy),
-                      Builder.getInt64(FunctionHash),
-                      Builder.getInt32(NumRegionCounters),
-                      Builder.getInt32(Counter)});
+
+  ArrayRef<llvm::Value *> Args = {
+      llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy),
+      Builder.getInt64(FunctionHash), Builder.getInt32(NumRegionCounters),
+      Builder.getInt32(Counter), StepV};
+  if (!StepV)
+    Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::instrprof_increment),
+                       Args.drop_back(1));
+  else
+    Builder.CreateCall(
+        CGM.getIntrinsic(llvm::Intrinsic::instrprof_increment_step), Args);
 }
 
 // This method either inserts a call to the profile run-time during

diff --git a/lib/CodeGen/CodeGenPGO.h b/lib/CodeGen/CodeGenPGO.h
index 4f229cde63b03426ae7f74b7a110422e4882b60c..0026df570bce0617034fef73833b71e9ad4f972d 100644 (file)
--- a/lib/CodeGen/CodeGenPGO.h
+++ b/lib/CodeGen/CodeGenPGO.h
@@ -105,7 +105,8 @@ private:
   void emitCounterRegionMapping(const Decl *D);
 
 public:
-  void emitCounterIncrement(CGBuilderTy &Builder, const Stmt *S);
+  void emitCounterIncrement(CGBuilderTy &Builder, const Stmt *S,
+                            llvm::Value *StepV);
 
   /// Return the region count for the counter at the given index.
   uint64_t getRegionCount(const Stmt *S) {

diff --git a/test/Profile/c-ternary.c b/test/Profile/c-ternary.c
new file mode 100644 (file)
index 0000000..09e8329
--- /dev/null
+++ b/test/Profile/c-ternary.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.11.0 -x c %s -o - -emit-llvm -fprofile-instrument=clang | FileCheck %s
+
+// PR32019: Clang can lower some ternary operator expressions to select
+// instructions. Make sure we only increment the profile counter for the
+// condition when the condition evaluates to true.
+// CHECK-LABEL: define i32 @f1
+int f1(int x) {
+// CHECK: [[TOBOOL:%.*]] = icmp ne i32 %1, 0
+// CHECK-NEXT: [[STEP:%.*]] = zext i1 [[TOBOOL]] to i64
+// CHECK-NEXT: [[COUNTER:%.*]] = load i64, i64* getelementptr inbounds ([2 x i64], [2 x i64]* @__profc_f1, i64 0, i64 1)
+// CHECK-NEXT: add i64 [[COUNTER]], [[STEP]]
+// CHECK: [[COND:%.*]] = select i1 [[TOBOOL]], i32 0, i32 1
+  return x ? 0 : 1;
+// CHECK: ret i32 [[COND]]
+}