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

Vedant Kumar via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 24 18:30:04 PST 2017


Author: vedantk
Date: Fri Feb 24 20:30:03 2017
New Revision: 296231

URL: http://llvm.org/viewvc/llvm-project?rev=296231&view=rev
Log:
[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.

Added:
    cfe/trunk/test/Profile/c-ternary.c
Modified:
    cfe/trunk/lib/CodeGen/CGExprScalar.cpp
    cfe/trunk/lib/CodeGen/CodeGenFunction.h
    cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
    cfe/trunk/lib/CodeGen/CodeGenPGO.h

Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=296231&r1=296230&r2=296231&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Fri Feb 24 20:30:03 2017
@@ -3414,9 +3414,11 @@ VisitAbstractConditionalOperator(const A
   // 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) {

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=296231&r1=296230&r2=296231&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Feb 24 20:30:03 2017
@@ -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);
   }
 

Modified: cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenPGO.cpp?rev=296231&r1=296230&r2=296231&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenPGO.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenPGO.cpp Fri Feb 24 20:30:03 2017
@@ -739,7 +739,8 @@ CodeGenPGO::applyFunctionAttributes(llvm
   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(CG
 
   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

Modified: cfe/trunk/lib/CodeGen/CodeGenPGO.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenPGO.h?rev=296231&r1=296230&r2=296231&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenPGO.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenPGO.h Fri Feb 24 20:30:03 2017
@@ -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) {

Added: cfe/trunk/test/Profile/c-ternary.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/c-ternary.c?rev=296231&view=auto
==============================================================================
--- cfe/trunk/test/Profile/c-ternary.c (added)
+++ cfe/trunk/test/Profile/c-ternary.c Fri Feb 24 20:30:03 2017
@@ -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]]
+}




More information about the cfe-commits mailing list