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

Fri Nov 6 18:05:01 PST 2015

> On 2015-Nov-05, at 22:17, Xinliang David Li <xinliangli at gmail.com> wrote:
> 
> I see. If will clean up the recent style violations introduced by me soon.

I don't think it's super important to go back and fix (although
appreciated if you get to it); the more important thing is to get
clang-format doing the right thing going forward!

> 
> thanks,
> 
> David
> 
> 
> On Thu, Nov 5, 2015 at 9:37 PM, Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> > On 2015-Oct-28, at 18:29, Xinliang David Li via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> >
> >
> >
> > On Wed, Oct 28, 2015 at 4:01 PM, Justin Bogner <mail at justinbogner.com> wrote:
> > 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?
> >
> > Could be that. I checked the latest clang-format-diff, it does produce the single line output. In fact, I see both styles are used widely in LLVM, so I guess they are both acceptable.
> 
> Both styles are widely used just because we have a lot of code that
> hasn't been clang-formatted.
> 
> Note that Teresa had the same configuration issue: for some reason
> (that neither of us could figure out), clang-format was using the
> Google style on her laptop instead of the LLVM style.  I noticed it
> because of the same quirk: `return`s on the same line.
> 
> Since the whole point of clang-format is to (over time) canonicalize
> the whitespace style, maybe Teresa (or Daniel!) can help you
> configure correctly for LLVM.
> 
> >
> > David
> >
> >
> > > 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;
> > >>
> > >>
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 