[PATCH] Don't instrument inline functions that are not externally visible

[Gonzalo Brito Gadeschi]

>
> What's special about non-externally visible inline functions?


They are not available to other translation units. However, if you take a
pointer to a non-externally visible inline function the compiler should not
remove the function. That is, even if the instrumentation call backs are
part of a different TU, passing a function pointer to them should just
work.

If you look at if it's inline specified in the frontend, that doesn't tell
> you if it actually gets inlined.


Indeed. Furthermore, AFAIK the instrumentation callbacks are pushed before
and after the function call site. That is, it doesn't matter if the
function gets inlined or not.

And finally, what's wrong with the linker such that it can't resolve the
> address of an internal inline function?  That seems like a perfectly
> reasonable thing for it to be able to do.  If you take the address of a
> function, the compiler should make sure not to delete the definition.
>

You're right. Passing a function pointer to the instrumentation call backs
prevents the compiler from removing the function. That is, the linker can
resolve the address in all cases.

This patch still doesn't make sense to me.


Indeed. Thank you a lot for taking your time to review it.


On Tue, Dec 3, 2013 at 7:35 PM, Reid Kleckner <rnk at google.com> wrote:

>  I made sure to CC your directly on the reply, FWIW.
>
>  This patch still doesn't make sense to me.  What's special about
> non-externally visible inline functions?  If you look at if it's inline
> specified in the frontend, that doesn't tell you if it actually gets
> inlined.  Why doesn't this apply to static functions that get inlined and
> aren't inline-specified?
>
>  And finally, what's wrong with the linker such that it can't resolve the
> address of an internal inline function?  That seems like a perfectly
> reasonable thing for it to be able to do.  If you take the address of a
> function, the compiler should make sure not to delete the definition.
>
>
> On Tue, Dec 3, 2013 at 3:05 AM, Gonzalo Brito Gadeschi <
> g.brito at aia.rwth-aachen.de> wrote:
>
>>  From: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131125/094256.html
>>
>>
>>
>>
>>
>>>
>>>
>>>
>>>
>>> I've tried to ask you twice to explain why this is necessary:
>>>
>>
>> Sorry, I was not subscribed to the list and was not getting any replies. I should be able to reply from now on.
>>
>>
>>
>>> >* Can you elaborate on why the linker can't resolve references to an *internal
>>> inline function? That seems like a perfectly reasonable thing for
>>> it to be able to do.
>>
>>
>>
>> "Inline functions that are not externally visible mustn't be instrumented. They create a named reference to the inlined function, as the first parameter to __cyg_profile_* functions, which a linker will never be able to resolve."
>>
>>  --
>> Dipl.-Ing. Gonzalo Brito Gadeschi
>> Institute of Aerodynamics and Chair of Fluid Mechanics
>> RWTH Aachen University
>> Wuellnerstraße 5a
>> D-52062 Aachen
>> Germany
>> Phone: ++49-(0)241-80-94821
>> Fax: ++49-(0)241-80-92257
>> E-mail:  g.brito at aia.rwth-aachen.de
>> Internet: www.aia.rwth-aachen.de
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>


-- 
Dipl.-Ing. Gonzalo Brito Gadeschi
Institute of Aerodynamics and Chair of Fluid Mechanics
RWTH Aachen University
Wuellnerstraße 5a
D-52062 Aachen
Germany
Phone: ++49-(0)241-80-94821
Fax: ++49-(0)241-80-92257
E-mail:  g.brito at aia.rwth-aachen.de
Internet: www.aia.rwth-aachen.de
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131204/962412fa/attachment.html>