[llvm-dev] [InstrProfiling] Lightweight Instrumentation

Mon Nov 8 11:44:17 PST 2021

ok thanks. SGTM.

On Mon, Oct 25, 2021 at 11:44 AM Ellis Hoag <ellis.sparky.hoag at gmail.com>
wrote:

> While I am not familiar with the PDB format, most of the changes are
> independent of the debug info format type. For the instrumentation
> counters, we emit a `!DIGlobalVariable()` in the LLVM IR which is emitted
> as Dwarf or CodeView automatically. That being said, I was planning on
> first extending the `llvm-profdata` tool to handle Dwarf and then I can add
> support for CodeView if needed.
>
> On Mon, Oct 25, 2021 at 12:58 PM Petr Hosek <phosek at google.com> wrote:
>
>> From our perspective, using debug info wouldn't be an issue since we
>> always include it in every build.
>>
>> The issue that hasn't been brought up yet is that the proposed solution
>> only covers DWARF, but the IR instrumentation also supports Windows which
>> uses PDB. More generally, the existing implementation is independent of the
>> debugging format being used. I'm not familiar with PDB to say whether the
>> proposed solution could be extended to also support Windows, but if not
>> then we'll have to support both the use of __llvm_prf_data and
>> __llvm_prf_names sections as well as debug info which seems like an extra
>> complexity.
>>
>> I agree with David that making __llvm_prf_names strippable and cutting
>> down the size of __llvm_prf_data would be a worthwhile effort independently
>> of the proposed changes.
>>
>> On Wed, Oct 20, 2021 at 10:00 AM Xinliang David Li <davidxl at google.com>
>> wrote:
>>
>>>
>>>
>>> On Tue, Oct 19, 2021 at 5:12 PM Wenlei He <wenlei at fb.com> wrote:
>>>
>>>> > While dwarf is a standard way of program annotation, using it for
>>>> instrumentation PGO does mean an additional dependency (instead of being
>>>> self contained).
>>>>
>>>>
>>>>
>>>> I think your point on having debug info as dependency is fair. But
>>>> given most release builds need to generate debug info anyways, this
>>>> dependency seems to be a minor downside. Hence the tradeoff between extra
>>>> dependency and minimum binary size can be worthwhile. In the end, this does
>>>> not “regress” an existing feature by adding extra dependency – i.e. it
>>>> doesn’t affect today’s IRPGO; and the actual dependency, the debug info is
>>>> already commonly used. Hope this is an acceptable trade-off.
>>>>
>>>>
>>>>
>>>> > This proposal requires debug_type info to be emitted, right? What is
>>>> the object size and compile time overhead? If this can be trimmed, it is a
>>>> reasonable way to emit the profile data mapping information at compile time.
>>>>
>>>>
>>>>
>>>> We don’t really require debug_type, so yes it can be trimmed as a
>>>> dependency. However, in practice whether we should trim it also depends on
>>>> whether it makes sense from dwarf perspective, and if it’s worth a new
>>>> mode/variant for dwarf info. I tend it view the trimming of debug_type as
>>>> orthogonal to this proposal – as long as we keep the lightweight PGO
>>>> technically independent of debug types, the trimming can be done later if
>>>> it’s appropriate from debug info perspective and when the actual use case
>>>> arises.
>>>>
>>>
>>>
>>> Thanks.  +Vedant and +Petr for their opinion on using debug info for
>>> matching purposes.
>>>
>>> David
>>>
>>>
>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Wenlei
>>>>
>>>>
>>>>
>>>> *From: *Xinliang David Li <davidxl at google.com>
>>>> *Date: *Monday, October 18, 2021 at 2:20 PM
>>>> *To: *Wenlei He <wenlei at fb.com>
>>>> *Cc: *Ellis Hoag <ellis.sparky.hoag at gmail.com>, Kyungwoo Lee <
>>>> kyulee at fb.com>, llvm-dev <llvm-dev at lists.llvm.org>
>>>> *Subject: *Re: [llvm-dev] [InstrProfiling] Lightweight Instrumentation
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On Mon, Oct 18, 2021 at 1:38 PM Wenlei He <wenlei at fb.com> wrote:
>>>>
>>>> Thanks for the feedback, David.
>>>>
>>>>
>>>>
>>>> You’re right that most of the savings comes from coarse-grained
>>>> instrumentation. However, the situation we’re facing for mobile (and also
>>>> embedded systems) comes with very tight size constraints. Some components
>>>> are already built with -Oz, and we’re constantly on the lookout for extra
>>>> MiB to save so more “features” can get into the components.
>>>>
>>>>
>>>>
>>>> For clang self-build example, 7M overhead is much better than 50M+, and
>>>> 50M->7M indeed look close to 50M->4M as improvements. But comparing to
>>>> non-PGO, this is still +7M vs +4M. The extra 3M is considered quite
>>>> significant, and could potentially be a deal breaker for some cases.
>>>>
>>>>
>>>>
>>>> In short, this is “close” as you mentioned, but not good enough still.
>>>> Using dwarf as metadata also has a few benefits over tweaking existing
>>>> metadata to be extractable: it’s less intrusive, and it’s also a more
>>>> standardized metadata comparing to PGO’s own metadata.
>>>>
>>>>
>>>>
>>>> While dwarf is a standard way of program annotation, using it for
>>>> instrumentation PGO does mean an additional dependency (instead of being
>>>> self contained).
>>>>
>>>>
>>>>
>>>> This proposal requires debug_type info to be emitted, right? What is
>>>> the object size and compile time overhead? If this can be trimmed, it is a
>>>> reasonable way to emit the profile data mapping information at compile time.
>>>>
>>>>
>>>>
>>>> David
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Wenlei
>>>>
>>>>
>>>>
>>>> *From: *llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of
>>>> Xinliang David Li via llvm-dev <llvm-dev at lists.llvm.org>
>>>> *Date: *Monday, October 18, 2021 at 1:14 PM
>>>> *To: *Ellis Hoag <ellis.sparky.hoag at gmail.com>
>>>> *Cc: *llvm-dev <llvm-dev at lists.llvm.org>
>>>> *Subject: *Re: [llvm-dev] [InstrProfiling] Lightweight Instrumentation
>>>>
>>>> Hi Ellis, thanks for the proposal. Improving the usability of
>>>> Instrumentation PGO is indeed very important.
>>>>
>>>>
>>>>
>>>> From the results data below, if I understand correctly, the main
>>>> savings are from supporting the coverage mode (using boolean counters),
>>>> right? If we only enable that, the meta data based IRPGO clang size will be
>>>> 10 MB larger (__llvm_prf_names are strippible or easily doable).
>>>>
>>>>
>>>>
>>>> About __llvm_prof_data -- it also serves the purpose of detecting CFG
>>>> mismatch (with cfg hashing). On the other hand, about half of the size is
>>>> used for value profiling purposes, so for coverage mode when value profile
>>>> is not needed, its size can be cut in half -- leaving the total overhead to
>>>> be roughly 7 MiB, very close to the debug info based matching scheme.
>>>>
>>>>
>>>>
>>>> I support the proposal related to different profiling modes (entry
>>>> only, boolean counter).  I suggest having those features upstreamed. In
>>>> addition, changes that can reduce existing IRPGO size (e.g, strippable name
>>>> section, reduced __llvm_prof_data) are also very welcome. After those are
>>>> done, we will have a better idea if the size is still an issue (with a
>>>> better comparison with the debug info based method).
>>>>
>>>>
>>>>
>>>> thanks,
>>>>
>>>>
>>>>
>>>> David
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20211108/34f3670d/attachment.html>