[PATCH] D18636: [PGO] Avoid instrumenting constants at value sites

[Justin Bogner via cfe-commits]

Betul Buyukkurt <betulb at codeaurora.org> writes:
> betulb created this revision.
> betulb added reviewers: davidxl, bogner.
> betulb added subscribers: llvm-commits, cfe-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".
>
> Value profiling should not profile constants and/or constant
> expressions when they appear as callees in call instructions. Constant
> expressions form when a direct callee has bitcasts or inttoptr(ptrtint
> (callee)) nests surrounding it. Value profiling should avoid
> instrumenting such cases. Mostly NFC.
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D18636
>
> Files:
>   lib/CodeGen/CodeGenPGO.cpp
>   test/Profile/c-avoid-direct-call.c
>
> Index: test/Profile/c-avoid-direct-call.c
> ===================================================================
> --- /dev/null
> +++ test/Profile/c-avoid-direct-call.c
> @@ -0,0 +1,11 @@
> +// Check the value profiling instrinsics emitted by instrumentation.
> +
> +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-avoid-direct-call.c %s -o - -emit-llvm -fprofile-instrument=clang -mllvm -enable-value-profiling | FileCheck %s
> +
> +void foo();
> +
> +int main(void) {
> +// CHECK-NOT: call void @__llvm_profile_instrument_target(i64 ptrtoint (void (...)* @foo to i64), i8* bitcast ({{.*}}* @__profd_main to i8*), i32 0)

This check seems too specific - I worry that the ptrtoint or the
bitcast could disappear for some reason and the CHECK-NOT will succeed
even if a __llvm_profile_instrument_target appears.

I think we can just go really fuzzy and say:

  // CHECK-NOT: call void @__llvm_profile_instrument_target

WDYT?

> +  foo(21);
> +  return 0;
> +}
> Index: lib/CodeGen/CodeGenPGO.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenPGO.cpp
> +++ lib/CodeGen/CodeGenPGO.cpp
> @@ -755,6 +755,9 @@
>    if (!ValuePtr || !ValueSite || !Builder.GetInsertBlock())
>      return;
>  
> +  if (dyn_cast<llvm::Constant>(ValuePtr))

Best to use isa<> rather than dyn_cast<> when you don't need to do
anything with the return value.

> +    return;
> +
>    bool InstrumentValueSites = CGM.getCodeGenOpts().hasProfileClangInstr();
>    if (InstrumentValueSites && RegionCounterMap) {
>      auto BuilderInsertPoint = Builder.saveIP();
>