[cfe-commits] [patch] Implement -finstrument-functions

[Nelson Elhage]

Attached. No worries about the misunderstanding. I added a comment
showing the type of the instrumentation functions, which will hopefully
make things slightly clearer.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: instrument-functions.diff
Type: text/x-diff
Size: 10890 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100622/96451d36/attachment.diff>
-------------- next part --------------


On Mon, 21 Jun 2010 17:07:17 -0700, Chris Lattner <clattner at apple.com> wrote:
> On Jun 19, 2010, at 4:34 PM, Nelson Elhage wrote:
> > Oops, I caught another bug in testing (apparently not caught by the test
> > suite -- I'll consider sending a separate patch to add a test): I was
> > accidentally causing all IgnoredAttribute's to turn into
> > no_instrument_function.  Revised patch attached.
> 
> Great, applied in r106507, just to get it out of your tree :)
> 
> > On Sat, 19 Jun 2010 13:01:11 -0400, Nelson Elhage <nelhage at nelhage.com> wrote:
> >> Thanks for the comments! Revised patch is attached. There's one point I
> >> wasn't totally clear on, though, so this may need another revision:
> >> 
> >> On Fri, 18 Jun 2010 22:04:53 -0700, Chris Lattner <clattner at apple.com> wrote:
> >>> On Jun 18, 2010, at 8:41 PM, Nelson Elhage wrote:
> >>> 
> >>> 
> >>> +                      llvm::ConstantExpr::getBitCast(CurFn, PointerTy),
> >>> 
> >>> This can go away when you use CreateRuntimeFunction.
> >> 
> >> The instrumentation functions are typed as void (i8*, i8*). Since CurFn
> >> isn't an i8*, there needs to be a cast somewhere. My revised patch asks
> >> CreateRuntimeFunction for a __cyg_profile_func_* typed as void
> >> (typeof(CurFn), i8*), which it will bitcast for me. I assume this is
> >> what you were suggesting?
> >> 
> >> But that means that __cyg_profile_func_enter will end up declared as
> >> taking a pointer to whatever function type CodeGenFunction sees first,
> >> which seems to work fine, but doesn't seem correct. Should I be doing
> >> something to ensure that it gets initially declared with the correct
> >> type?
> 
> Ah, I see what you mean.  The old way you had it definitely is better.
> I didn't understand the purpose of the case.  Can you send in a patch
> on top of mainline that changes it back?  I'm sorry for not
> understanding.
> 
> Thanks for working on this!
> 
> -Chris