[PATCH] D40276: Add -finstrument-function-entry-bare flag

2017-11-21 Thread Hans Wennborg via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL318785: Add -finstrument-function-entry-bare flag (authored 
by hans).

Changed prior to commit:
  https://reviews.llvm.org/D40276?vs=123691=123821#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40276

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/instrument-functions.c

Index: cfe/trunk/test/CodeGen/instrument-functions.c
===
--- cfe/trunk/test/CodeGen/instrument-functions.c
+++ cfe/trunk/test/CodeGen/instrument-functions.c
@@ -1,21 +1,34 @@
 // RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-function-entry-bare -disable-llvm-passes | FileCheck -check-prefix=BARE %s
 
 int test1(int x) {
 // CHECK: @test1(i32 {{.*}}%x) #[[ATTR1:[0-9]+]]
 // CHECK: ret
+
+// BARE: @test1(i32 {{.*}}%x) #[[ATTR1:[0-9]+]]
+// BARE: ret
   return x;
 }
 
 int test2(int) __attribute__((no_instrument_function));
 int test2(int x) {
 // CHECK: @test2(i32 {{.*}}%x) #[[ATTR2:[0-9]+]]
 // CHECK: ret
+
+// BARE: @test2(i32 {{.*}}%x) #[[ATTR2:[0-9]+]]
+// BARE: ret
   return x;
 }
 
 // CHECK: attributes #[[ATTR1]] =
 // CHECK-SAME: "instrument-function-entry"="__cyg_profile_func_enter"
 // CHECK-SAME: "instrument-function-exit"="__cyg_profile_func_exit"
 
+// BARE: attributes #[[ATTR1]] =
+// BARE-SAME: "instrument-function-entry-inlined"="__cyg_profile_func_enter_bare"
+
 // CHECK: attributes #[[ATTR2]] =
 // CHECK-NOT: "instrument-function-entry"
+
+// BARE: attributes #[[ATTR2]] =
+// BARE-NOT: "instrument-function-entry"
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -782,6 +782,8 @@
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
   Opts.InstrumentFunctionsAfterInlining =
   Args.hasArg(OPT_finstrument_functions_after_inlining);
+  Opts.InstrumentFunctionEntryBare =
+  Args.hasArg(OPT_finstrument_function_entry_bare);
   Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
@@ -355,10 +355,11 @@
   llvm::DebugLoc Loc = EmitReturnBlock();
 
   if (ShouldInstrumentFunction()) {
-CurFn->addFnAttr(!CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining
- ? "instrument-function-exit"
- : "instrument-function-exit-inlined",
- "__cyg_profile_func_exit");
+if (CGM.getCodeGenOpts().InstrumentFunctions)
+  CurFn->addFnAttr("instrument-function-exit", "__cyg_profile_func_exit");
+if (CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining)
+  CurFn->addFnAttr("instrument-function-exit-inlined",
+   "__cyg_profile_func_exit");
   }
 
   // Emit debug descriptor for function end.
@@ -443,7 +444,8 @@
 /// instrumented with __cyg_profile_func_* calls
 bool CodeGenFunction::ShouldInstrumentFunction() {
   if (!CGM.getCodeGenOpts().InstrumentFunctions &&
-  !CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining)
+  !CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining &&
+  !CGM.getCodeGenOpts().InstrumentFunctionEntryBare)
 return false;
   if (!CurFuncDecl || CurFuncDecl->hasAttr())
 return false;
@@ -982,10 +984,14 @@
   }
 
   if (ShouldInstrumentFunction()) {
-Fn->addFnAttr(!CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining
-  ? "instrument-function-entry"
-  : "instrument-function-entry-inlined",
-  "__cyg_profile_func_enter");
+if (CGM.getCodeGenOpts().InstrumentFunctions)
+  CurFn->addFnAttr("instrument-function-entry", "__cyg_profile_func_enter");
+if (CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining)
+  CurFn->addFnAttr("instrument-function-entry-inlined",
+   "__cyg_profile_func_enter");
+if (CGM.getCodeGenOpts().InstrumentFunctionEntryBare)
+  CurFn->addFnAttr("instrument-function-entry-inlined",
+   "__cyg_profile_func_enter_bare");
   }
 
   // Since emitting the mcount call here impacts optimizations such as function
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp

[PATCH] D40276: Add -finstrument-function-entry-bare flag

2017-11-21 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.

Yep, looks good. :)


https://reviews.llvm.org/D40276



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40276: Add -finstrument-function-entry-bare flag

2017-11-21 Thread Hans Wennborg via Phabricator via cfe-commits 
hans added a comment.

Reid, are you happy with this too?

In https://reviews.llvm.org/D40276#931502, @pasko wrote:

> Instrumenting the function entry post-inlining, without function exit, and 
> with no parameters is exactly what we need. The 
> `__cyg_profile_func_enter_bare` sounds good to me as a name. Thank you!


Great!

> Unnecessary thoughts just to get a feeling we are on the same page: this 
> could theoretically be made more orthogonal where 
> `-finstrument-functions-after-inlining` could regulate whether the call is 
> pre- or post-inlining, but I don't see how pre-inlining without parameters 
> would be usable without too much DWARF digging, which is not too practical.

The way I think about them, these flags all enable separate instrumentations 
(though you can only enable one at a time), they don't modify eachother as I 
think that might end up messier for the user.

In https://reviews.llvm.org/D40276#931706, @mattcary wrote:

> It looks like there also has to be a change to 
> llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp?


Yes, I'll land that in llvm once we're happy with this. I just wanted to make 
sure we get the flag and function names right, then the rest is trivial.


https://reviews.llvm.org/D40276



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40276: Add -finstrument-function-entry-bare flag

2017-11-21 Thread Matthew Cary via Phabricator via cfe-commits 
mattcary added a comment.

It looks like there also has to be a change to 
llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp?

Index: lib/Transforms/Utils/EntryExitInstrumenter.cpp
=

- lib/Transforms/Utils/EntryExitInstrumenter.cpp(revision 318760)

+++ lib/Transforms/Utils/EntryExitInstrumenter.cpp  (working copy)
@@ -51,6 +51,10 @@

  CallInst::Create(Fn, ArrayRef(Args), "", InsertionPt);
  return;

+  } else if (Func == "__cyg_profile_func_enter_bare") {
+Constant *Fn = M.getOrInsertFunction(Func, Type::getVoidTy(C));
+CallInst::Create(Fn, "", InsertionPt);
+return;

  }
   
  // We only know how to call a fixed set of instrumentation functions, because


https://reviews.llvm.org/D40276



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40276: Add -finstrument-function-entry-bare flag

2017-11-21 Thread Egor Pasko via Phabricator via cfe-commits 
pasko accepted this revision.
pasko added a comment.
This revision is now accepted and ready to land.

Instrumenting the function entry post-inlining, without function exit, and with 
no parameters is exactly what we need. The `__cyg_profile_func_enter_bare` 
sounds good to me as a name. Thank you!

Unnecessary thoughts just to get a feeling we are on the same page: this could 
theoretically be made more orthogonal where 
`-finstrument-functions-after-inlining` could regulate whether the call is pre- 
or post-inlining, but I don't see how pre-inlining without parameters would be 
usable without too much DWARF digging, which is not too practical.


https://reviews.llvm.org/D40276



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40276: Add -finstrument-function-entry-bare flag

2017-11-20 Thread Hans Wennborg via Phabricator via cfe-commits 
hans created this revision.

This is an instrumentation flag, similar to -finstrument-functions, but it only 
inserts calls on function entry, the calls are inserted post-inlining, and they 
don't take any arugments.

This is intended for users who want to instrument function entry with minimal 
overhead.

(-pg would be another alternative, but foces frame pointer emission and affects 
linking, so is probably best left alone to be used for generating gcov data.)


https://reviews.llvm.org/D40276

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/instrument-functions.c

Index: test/CodeGen/instrument-functions.c
===
--- test/CodeGen/instrument-functions.c
+++ test/CodeGen/instrument-functions.c
@@ -1,21 +1,34 @@
 // RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -S -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-function-entry-bare -disable-llvm-passes | FileCheck -check-prefix=BARE %s
 
 int test1(int x) {
 // CHECK: @test1(i32 {{.*}}%x) #[[ATTR1:[0-9]+]]
 // CHECK: ret
+
+// BARE: @test1(i32 {{.*}}%x) #[[ATTR1:[0-9]+]]
+// BARE: ret
   return x;
 }
 
 int test2(int) __attribute__((no_instrument_function));
 int test2(int x) {
 // CHECK: @test2(i32 {{.*}}%x) #[[ATTR2:[0-9]+]]
 // CHECK: ret
+
+// BARE: @test2(i32 {{.*}}%x) #[[ATTR2:[0-9]+]]
+// BARE: ret
   return x;
 }
 
 // CHECK: attributes #[[ATTR1]] =
 // CHECK-SAME: "instrument-function-entry"="__cyg_profile_func_enter"
 // CHECK-SAME: "instrument-function-exit"="__cyg_profile_func_exit"
 
+// BARE: attributes #[[ATTR1]] =
+// BARE-SAME: "instrument-function-entry-inlined"="__cyg_profile_func_enter_bare"
+
 // CHECK: attributes #[[ATTR2]] =
 // CHECK-NOT: "instrument-function-entry"
+
+// BARE: attributes #[[ATTR2]] =
+// BARE-NOT: "instrument-function-entry"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -782,6 +782,8 @@
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
   Opts.InstrumentFunctionsAfterInlining =
   Args.hasArg(OPT_finstrument_functions_after_inlining);
+  Opts.InstrumentFunctionEntryBare =
+  Args.hasArg(OPT_finstrument_function_entry_bare);
   Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3554,8 +3554,11 @@
 options::OPT_fno_unique_section_names, true))
 CmdArgs.push_back("-fno-unique-section-names");
 
-  Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
-  options::OPT_finstrument_functions_after_inlining);
+  if (auto *A = Args.getLastArg(
+  options::OPT_finstrument_functions,
+  options::OPT_finstrument_functions_after_inlining,
+  options::OPT_finstrument_function_entry_bare))
+A->render(Args, CmdArgs);
 
   addPGOAndCoverageFlags(C, D, Output, Args, CmdArgs);
 
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -355,10 +355,11 @@
   llvm::DebugLoc Loc = EmitReturnBlock();
 
   if (ShouldInstrumentFunction()) {
-CurFn->addFnAttr(!CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining
- ? "instrument-function-exit"
- : "instrument-function-exit-inlined",
- "__cyg_profile_func_exit");
+if (CGM.getCodeGenOpts().InstrumentFunctions)
+  CurFn->addFnAttr("instrument-function-exit", "__cyg_profile_func_exit");
+if (CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining)
+  CurFn->addFnAttr("instrument-function-exit-inlined",
+   "__cyg_profile_func_exit");
   }
 
   // Emit debug descriptor for function end.
@@ -443,7 +444,8 @@
 /// instrumented with __cyg_profile_func_* calls
 bool CodeGenFunction::ShouldInstrumentFunction() {
   if (!CGM.getCodeGenOpts().InstrumentFunctions &&
-  !CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining)
+  !CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining &&
+  !CGM.getCodeGenOpts().InstrumentFunctionEntryBare)
 return false;
   if (!CurFuncDecl || CurFuncDecl->hasAttr())
 return false;
@@ -982,10 +984,14 @@
   }
 
   if (ShouldInstrumentFunction()) {
-Fn->addFnAttr(!CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining
-  ?