[PATCH] D18569: [PGO] Move the instrumentation point closer to the value site.

[Justin Bogner via llvm-commits]

Betul Buyukkurt <betulb at codeaurora.org> writes:
> betulb created this revision.
> betulb added reviewers: davidxl, bogner, xur.
> betulb added a subscriber: llvm-commits.
> betulb set the repository for this revision to rL LLVM.
> betulb changed the visibility of this Differential Revision from
> "Public (No Login Required)" to "All Users".
>
> For terminator instructions, the value profiling instrumentation
> happens in a basic block other than where the value site resides. This
> CR moves the instrumentation point prior to the value site. Mostly
> NFC.
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D18569
>
> Files:
>   lib/CodeGen/CodeGenPGO.cpp
>   test/Profile/c-indirect-call.c
>   test/Profile/cxx-indirect-call.cpp
>
> Index: test/Profile/cxx-indirect-call.cpp
> ===================================================================
> --- /dev/null
> +++ test/Profile/cxx-indirect-call.cpp
> @@ -0,0 +1,21 @@
> +// Check the value profiling instrinsics emitted by instrumentation.
> +
> +// RUN: %clangxx %s -o - -emit-llvm -S -fprofile-instr-generate -mllvm -enable-value-profiling -fexceptions -target %itanium_abi_triple | FileCheck %s
> +
> +void (*foo) (void);
> +
> +int main(int argc, const char *argv[]) {
> +// CHECK:  [[REG1:%[0-9]+]] = load void ()*, void ()** @foo, align 4
> +// CHECK-NEXT:  [[REG2:%[0-9]+]] = ptrtoint void ()* [[REG1]] to i64
> +// CHECK-NEXT:  call void @__llvm_profile_instrument_target(i64 [[REG2]], i8* bitcast ({{.*}}* @__profd_main to i8*), i32 0)
> +// CHECK-NEXT:  invoke void [[REG1]]()
> +  try {
> +    foo();
> +  } catch (int) {}
> +  return 0;
> +}
> +
> +// CHECK: declare void @__llvm_profile_instrument_target(i64, i8*, i32)
> +
> +
> +
> Index: test/Profile/c-indirect-call.c
> ===================================================================
> --- test/Profile/c-indirect-call.c
> +++ test/Profile/c-indirect-call.c
> @@ -1,13 +1,14 @@
> -// Check the data structures emitted by instrumentation.
> +// Check the value profiling instrinsics emitted by instrumentation.
> +
>  // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-indirect-call.c %s -o - -emit-llvm -fprofile-instrument=clang -mllvm -enable-value-profiling | FileCheck %s
>  
>  void (*foo)(void);
>  
>  int main(void) {
>  // CHECK:  [[REG1:%[0-9]+]] = load void ()*, void ()** @foo, align 8
> -// CHECK-NEXT:  call void [[REG1]]()
>  // CHECK-NEXT:  [[REG2:%[0-9]+]] = ptrtoint void ()* [[REG1]] to i64
>  // CHECK-NEXT:  call void @__llvm_profile_instrument_target(i64 [[REG2]], i8* bitcast ({{.*}}* @__profd_main to i8*), i32 0)
> +// CHECK-NEXT:  call void [[REG1]]()
>    foo();
>    return 0;
>  }
> Index: lib/CodeGen/CodeGenPGO.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenPGO.cpp
> +++ lib/CodeGen/CodeGenPGO.cpp
> @@ -757,17 +757,18 @@
>  
>    bool InstrumentValueSites = CGM.getCodeGenOpts().hasProfileClangInstr();
>    if (InstrumentValueSites && RegionCounterMap) {
> -    llvm::LLVMContext &Ctx = CGM.getLLVMContext();
> -    auto *I8PtrTy = llvm::Type::getInt8PtrTy(Ctx);
> +    auto BuilderInsertPoint = Builder.saveIP();
> +    Builder.SetInsertPoint(ValueSite);
>      llvm::Value *Args[5] = {
> -        llvm::ConstantExpr::getBitCast(FuncNameVar, I8PtrTy),
> +        llvm::ConstantExpr::getBitCast(FuncNameVar, Builder.getInt8PtrTy()),
>          Builder.getInt64(FunctionHash),
>          Builder.CreatePtrToInt(ValuePtr, Builder.getInt64Ty()),
>          Builder.getInt32(ValueKind),
>          Builder.getInt32(NumValueSites[ValueKind]++)
>      };
>      Builder.CreateCall(
>          CGM.getIntrinsic(llvm::Intrinsic::instrprof_value_profile), Args);
> +    Builder.restoreIP(BuilderInsertPoint);
>      return;
>    }
>  

LGTM.