[cfe-commits] [patch] Implement -finstrument-functions
Nelson Elhage
nelhage at nelhage.com
Tue Jun 22 18:51:01 PDT 2010
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
More information about the cfe-commits
mailing list