[PATCH] D14033: [PGO] Eliminate emission of runtime hook user (Linux)

[Justin Bogner via llvm-commits]

Xinliang David Li <davidxl at google.com> writes:
> Regarding the formatting of return statement -- it is the
> clang-format-diff.py tool that does that for me (which I always run).

Strange. clang-format-diff.py on my machine puts the return on a new
line. Maybe clang-format's llvm-style changed at some point?

> David
>
> On Wed, Oct 28, 2015 at 11:29 AM, Justin Bogner <mail at justinbogner.com>
> wrote:
>
>> David Li <davidxl at google.com> writes:
>> > davidxl created this revision.
>> > davidxl added a reviewer: bogner.
>> > davidxl added a subscriber: llvm-commits.
>> >
>> > This is patch part-2 following: http://reviews.llvm.org/D14030
>> >
>> > http://reviews.llvm.org/D14033
>> >
>> > Files:
>> >   lib/Transforms/Instrumentation/InstrProfiling.cpp
>> >   test/Instrumentation/InstrProfiling/linkage.ll
>> >
>> > Index: test/Instrumentation/InstrProfiling/linkage.ll
>> > ===================================================================
>> > --- test/Instrumentation/InstrProfiling/linkage.ll
>> > +++ test/Instrumentation/InstrProfiling/linkage.ll
>> > @@ -1,7 +1,7 @@
>> >  ;; Check that runtime symbols get appropriate linkage.
>> >
>> >  ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -instrprof -S |
>> FileCheck %s
>> > -; RUN: opt < %s -mtriple=x86_64-unknown-linux -instrprof -S | FileCheck
>> %s
>> > +; RUN: opt < %s -mtriple=x86_64-unknown-linux -instrprof -S | FileCheck
>> %s --check-prefix=LINUX
>> >
>> >  @__llvm_profile_name_foo = hidden constant [3 x i8] c"foo"
>> >  @__llvm_profile_name_foo_weak = weak hidden constant [8 x i8]
>> c"foo_weak"
>> > @@ -10,6 +10,8 @@
>> >
>> >  ; CHECK: @__llvm_profile_counters_foo = hidden global
>> >  ; CHECK: @__llvm_profile_data_foo = hidden constant
>> > +; LINUX: @__llvm_profile_counters_foo = hidden global
>> > +; LINUX: @__llvm_profile_data_foo = hidden constant
>>
>> You can simplify the checks a bit by using multiple prefices - FileCheck
>> will check all of them. So you'd change these to "COMMON:" or something
>> and then have specific ones for emitting and skipping the runtime hook
>> definition below.
>>
>> >  define void @foo() {
>> >    call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x
>> i8], [3 x i8]* @__llvm_profile_name_foo, i32 0, i32 0), i64 0, i32 1, i32 0)
>> >    ret void
>> > @@ -17,6 +19,8 @@
>> >
>> >  ; CHECK: @__llvm_profile_counters_foo_weak = weak hidden global
>> >  ; CHECK: @__llvm_profile_data_foo_weak = weak hidden constant
>> > +; LINUX: @__llvm_profile_counters_foo_weak = weak hidden global
>> > +; LINUX: @__llvm_profile_data_foo_weak = weak hidden constant
>> >  define weak void @foo_weak() {
>> >    call void @llvm.instrprof.increment(i8* getelementptr inbounds ([8 x
>> i8], [8 x i8]* @__llvm_profile_name_foo_weak, i32 0, i32 0), i64 0, i32 1,
>> i32 0)
>> >    ret void
>> > @@ -24,6 +28,8 @@
>> >
>> >  ; CHECK: @"__llvm_profile_counters_linkage.ll:foo_internal" = internal
>> global
>> >  ; CHECK: @"__llvm_profile_data_linkage.ll:foo_internal" = internal
>> constant
>> > +; LINUX: @"__llvm_profile_counters_linkage.ll:foo_internal" = internal
>> global
>> > +; LINUX: @"__llvm_profile_data_linkage.ll:foo_internal" = internal
>> constant
>> >  define internal void @foo_internal() {
>> >    call void @llvm.instrprof.increment(i8* getelementptr inbounds ([23 x
>> i8], [23 x i8]* @"__llvm_profile_name_linkage.ll:foo_internal", i32 0, i32
>> 0), i64 0, i32 1, i32 0)
>> >    ret void
>> > @@ -31,6 +37,8 @@
>> >
>> >  ; CHECK: @__llvm_profile_counters_foo_inline = linkonce_odr hidden
>> global
>> >  ; CHECK: @__llvm_profile_data_foo_inline = linkonce_odr hidden constant
>> > +; LINUX: @__llvm_profile_counters_foo_inline = linkonce_odr hidden
>> global
>> > +; LINUX: @__llvm_profile_data_foo_inline = linkonce_odr hidden constant
>> >  define linkonce_odr void @foo_inline() {
>> >    call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x
>> i8], [10 x i8]* @__llvm_profile_name_foo_inline, i32 0, i32 0), i64 0, i32
>> 1, i32 0)
>> >    ret void
>> > @@ -39,8 +47,11 @@
>> >  declare void @llvm.instrprof.increment(i8*, i64, i32, i32)
>> >
>> >  ; CHECK: @__llvm_profile_runtime = external global i32
>> > +; LINUX-NOT: @__llvm_profile_runtime = external global i32
>> >
>> >  ; CHECK: define linkonce_odr hidden i32 @__llvm_profile_runtime_user()
>> {{.*}} {
>> >  ; CHECK:   %[[REG:.*]] = load i32, i32* @__llvm_profile_runtime
>> >  ; CHECK:   ret i32 %[[REG]]
>> >  ; CHECK: }
>> > +; LINUX-NOT: define linkonce_odr hidden i32
>> @__llvm_profile_runtime_user() {{.*}} {
>> > +; LINUX-NOT:   %[[REG:.*]] = load i32, i32* @__llvm_profile_runtime
>> > Index: lib/Transforms/Instrumentation/InstrProfiling.cpp
>> > ===================================================================
>> > --- lib/Transforms/Instrumentation/InstrProfiling.cpp
>> > +++ lib/Transforms/Instrumentation/InstrProfiling.cpp
>> > @@ -288,6 +288,8 @@
>> >
>> >  void InstrProfiling::emitRuntimeHook() {
>> >
>> > +  if (Triple(M->getTargetTriple()).isOSLinux()) return;
>> > +
>>
>> This could use a comment. Also, despite the formatting on the line
>> below, the style is to put the return on a new line. clang-format will
>> do that for you.
>>
>> >    // If the module's provided its own runtime, we don't need to do
>> anything.
>> >    if (M->getGlobalVariable(getInstrProfRuntimeHookVarName())) return;
>>
>>