[llvm-dev] The state of IRPGO (3 remaining work items)

[Eric Christopher via llvm-dev]

On Sun, Jun 26, 2016 at 11:04 PM Xinliang David Li <davidxl at google.com>
wrote:

> On Sun, Jun 26, 2016 at 10:21 PM, Dean Michael Berris <dberris at google.com>
> wrote:
>
>>
>>
>> On Mon, Jun 27, 2016 at 2:53 PM Xinliang David Li <davidxl at google.com>
>> wrote:
>>
>>> There is some misunderstanding about the intention of this flag.  The
>>> purpose of the flag is not to turn on profile instrumentation (which
>>> already has -fprofile-instr-generate or -fprofile-generate for it), but to
>>> select which instrumentors to use for PGO (IR or FE).  I prefer fewer flags
>>> too, but sharing flags for completely different purpose does not seem like
>>> the right thing to do.
>>>
>>>
>> Ah, right. It does seem like I'm misunderstanding the intention for that
>> flag.
>>
>> FWIW, I think consolidating the flags can happen later on, when we have a
>> better idea of how the different instrumentation pieces fit together.
>>
>
>
> I hope so :)   Note that PGO related instrumentation is not entirely the
> same as general profiling instrumentations. The former should also have a
> corresponding profile-use component in the compiler.  Current PGO
> instrumentation has edge/block profiling and a value profiler component.
> The FE based edge/block profiler is also shared with coverage testing.
>
>
>
>> Right now IIUC the only other thing that might count as an alternate
>> instrumentation implementation is XRay (and I'm not even sure they're
>> mutually exclusive either, i.e. both the profiling and XRay instrumentation
>> should be able to live together in the same binary).
>>
>
> Sanitizers (asan, tsan, ubsan, msan) are all program instrumenters. Asan
> also supports a mode for coverage testing. Profile related, we also have
> esan (efficiency sanitizer) under development.  The sanitizer options of
> course are unified in its own category.
>
>
Agreed. This all makes sense. Thanks David :)

-eric


> David
>
>
>
>
>
>> So if it's just the two that will be interacting, then sharing flags
>> doesn't seem worth it. But if I'm missing another, then the case for
>> consolidating the flags becomes stronger.
>>
>> Cheers
>>
>>
>>> David
>>>
>>>
>>> On Sun, Jun 26, 2016 at 9:39 PM, Dean Michael Berris <dberris at google.com
>>> > wrote:
>>>
>>>> On Fri, Jun 24, 2016 at 1:44 AM Eric Christopher via llvm-dev <
>>>> llvm-dev at lists.llvm.org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Thu, Jun 2, 2016, 6:41 PM Xinliang David Li via llvm-dev <
>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>
>>>>>>
>>>>>> Sounds fine to me, though I am not a fan of using unstable in the
>>>>>> option. I think a more meaningful way (that capture the essence of the
>>>>>> difference) is the following naming:
>>>>>> 1) FEPGO:  -fprofile-instr-generate=source or
>>>>>> -fprofile-instr-generate=region
>>>>>> 2) IR: -fprofile-instr-generate=cfg or -fprofile-instr-generate=graph
>>>>>>
>>>>>> Also since -fprofile-instr-generate= form is already used to specify
>>>>>> raw profile path, we may need a different driver option. Alternatives
>>>>>> include
>>>>>> 1) -fprofile-instrument=<...> -- this maps directly to the cc1 option
>>>>>> we have today
>>>>>> or
>>>>>> 2) -fpgo-instr=<> -- suggested by Fred or
>>>>>> 3) -fpgo-instr-method=<...>
>>>>>>
>>>>>
>>>>> Random bikeshedding. I like fprofile-instrument because it merges a
>>>>> lot of similar ideas behind instrumenting - and oddly enough what I was
>>>>> suggesting in the x-ray thread before seeing this.
>>>>>
>>>>>
>>>> +1 to -fprofile-instrument=... (as someone working on the XRay stuff,
>>>> I'd much rather have less flags, and consolidate a lot of these similar
>>>> things into a more inclusive flag).
>>>>
>>>> I would even make it shorter, and say -finstrument={profile-...,
>>>> xray-...} so we can have multiple "namespaced" values for -finstrument=.
>>>>
>>>> Just my A$0.02.
>>>>
>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160627/112c097a/attachment.html>