[llvm-commits] [patch] Implement a pass to instrument function entry and exit

[Nelson Elhage]

On Sat, 12 Jun 2010 21:51:54 -0700, Chris Lattner <clattner at apple.com> wrote:
> 
> Nice, adding -finstrument-functions support would be great.
> 
> Meta comment: please include your patch as an attachment.  If you're using Thunderbird, please follow these instructions:
> http://llvm.org/docs/DeveloperPolicy.html#patches
> 
> This makes it easier to apply.

Sorry about that, will fix in the future.

> 
> > +//===----------------------------------------------------------------------===//
> > +//
> > +// This pass inserts instrumentation calls for entry and exit to
> > +// functions. After function entry and before exit, calls are inserted
> > +// to the following instrumentation functions:
> > +//
> > +//        void __cyg_profile_func_enter (void *this_fn,
> > +//                                       void *call_site);
> > +//        void __cyg_profile_func_exit  (void *this_fn,
> > +//                                       void *call_site);
> > +//
> > +//===----------------------------------------------------------------------===//
> > +
> > +#define DEBUG_TYPE "instrument-functions"
> > +#include "llvm/CodeGen/Passes.h"
> 
> This should be added to the scalar passes.

Ok. I realized after sending that CodeGen was probably the wrong place,
but wasn't sure where it should go. I'll move it if we decide to go
foward with the LLVM pass approach.

> 
> > +namespace {
> > +  class CommaSeparatedStringParser :
> > +    public cl::basic_parser<std::vector<std::string> > {
> > +  public:
> > +    // parse - Return true on error.
> > +    bool parse(cl::Option &O, StringRef ArgName, StringRef ArgValue,
> > +               std::vector<std::string> &Val);
> > +  };
> 
> Instead of handling this with command line options, how about adding a
> function attribute?  That way the frontend can slap it on functions
> where it makes sense and manage the no_instrument_function attribute.

My intent was to have both the attribute and the command-line options;
These options would be used to implement
-finstrument-functions-exclude-{file,function}-list in the frontend. But
I can push those into clang and have it turn them into attributes, if
you think that's more appropriate.

> 
> However, backing up a little bit, I'm not sure I understand this from the GCC man page:
> 
> "This instrumentation is also done for functions expanded inline in other functions."
> 
> Does that mean that *all* source-level function (that don't have the
> no_instrument_function attribute) get instrumented, even if they get
> inlined etc?

That's correct.

(As of GCC 4, there is a complication, which is that as of GCC 4,
inlined functions call __cyg_profile_* with the address and caller of
the function _they were inlined into_, instead of passing the address of
the non-inlined version of the function. (See
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=23296). I'm not sure whether
we want to duplicate the GCC behavior, which I and at least two others
find confusing (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28205), or
do something else. But in any case, yes, all source-level functions get
instrumented).

> 
> If so, then I think this entire thing can be handled in the Clang IR
> generation routines, not needing LLVM IR attributes and not needing an
> LLVM IR pass.
> 
> What do you think?

What do you see as the advantage to handling this from clang, instead of
as an LLVM IR pass? My intuition would be that making this an IR pass is
desirable because you might want to instrument code generated by
something other than clang, but I don't yet have a strong feel for the
overall architecture and where the clang/LLVM split is.

- Nelson

> 
> -Chris
> 
>