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

[Chris Lattner]

Thanks Nelson,

Can you rediff this against mainline?  I already attached your previous patch, so I need the diff from it to this update.

-Chris

On Jun 22, 2010, at 6:51 PM, Nelson Elhage wrote:

> Attached. No worries about the misunderstanding. I added a comment
> showing the type of the instrumentation functions, which will hopefully
> make things slightly clearer.
> 
> <instrument-functions.diff>
> 
> 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