[PATCH] D35384: PGOInstrumentation: Move profile matching warnings to remarks

[Xinliang David Li via llvm-commits]

If a user is in panic mode, yes, they may use that. The use case I am
thinking of is for compiler developers. Developers working on PGO should
always turn on the error in their build and testing so that errors can be
caught easily. Another way is to add an validation pass but that seems like
an overkill given that the machinery is already there.  There will be a
usability regression without the force-error.

David

On Thu, Jul 20, 2017 at 10:30 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Thu, Jul 20, 2017 at 10:28 AM Xinliang David Li <davidxl at google.com>
> wrote:
>
>> I am fine making it remarks, but I think there needs to an option to
>> force it to be warning or even error.
>>
>
> Why's that? For users that are sure they're using a profile from exactly
> the same source code, so it should always line up? (do we have that use
> case internally? I'm not sure - certainly the motivation for this change
> was that it doesn't seem to make sense as a warning/error internally at
> least in some places, maybe not in all places?)
>
> Chandler: Thoughts? It doesn't seem totally crazy to me that we might have
> a -Rerror=X (and/or perhaps -Rwarning=X) in LLVM? Though it'd be pretty
> unusable for this without finer granularity (you could turn /all/
> optimization remarks into warnings or errors, but not more
> granular/specific than that, I think?)
>
>
>>
>> David
>>
>> On Thu, Jul 20, 2017 at 10:22 AM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>> Ping (Chandler and/or David)
>>>
>>> On Fri, Jul 14, 2017 at 7:15 PM David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>> On Thu, Jul 13, 2017 at 10:53 PM Xinliang David Li <davidxl at google.com>
>>>> wrote:
>>>>
>>>>> On Thu, Jul 13, 2017 at 10:41 PM, Chandler Carruth <
>>>>> chandlerc at google.com> wrote:
>>>>>
>>>>>> On Thu, Jul 13, 2017 at 7:29 PM Xinliang David Li via llvm-commits <
>>>>>> llvm-commits at lists.llvm.org> wrote:
>>>>>>
>>>>>>> On Thu, Jul 13, 2017 at 4:17 PM, David Blaikie <dblaikie at gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Thu, Jul 13, 2017 at 2:03 PM David Li via Phabricator <
>>>>>>>> reviews at reviews.llvm.org> wrote:
>>>>>>>>
>>>>>>>>> davidxl added a comment.
>>>>>>>>>
>>>>>>>>> Blindly downgrade the warning into remarks can be bad -- profile
>>>>>>>>> mismatch problems can go undetected. People may spend more time diagnosing
>>>>>>>>> performance regressions due to missed warnings. Worse, we may regress in
>>>>>>>>> compiler without noticing.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Any other ideas?
>>>>>>>>
>>>>>>>> The diagnostic handling callbacks through to Clang aren't granular
>>>>>>>> enough to demote only this warning to -Wno-error
>>>>>>>>
>>>>>>>
>>>>>>> Can this be fixed?
>>>>>>>
>>>>>>
>>>> Maybe, though I imagine to do that would mean plumbing this diagnostic
>>>> as a first class/separately identified one in the callback system, which I
>>>> would guess/assume we try not to do because it increases the
>>>> coupling/surface area between LLVM and Clang and the optimizers and
>>>> diagnostic infrastructure.
>>>>
>>>> In any case, even if we could, we'd still be left in the same place of
>>>> telling users basically "this diagnostic isn't high enough quality to
>>>> -Werror it, you should turn it back into a warning-not-error" and at that
>>>> point, we know from experience, people tend to to ignore them...
>>>>
>>>>
>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> (also as a warning it undermines the confidence as warnings as
>>>>>>>> actionable things that Clang has helped provide users).
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Warnings like this are usually actionable by users or compiler
>>>>>>> developers.
>>>>>>>
>>>>>>
>>>>>> Warnings are really to indicate a bug in user code.
>>>>>>
>>>>>> For example, we use some things like this to warn users when inline
>>>>>> assembly is likely to have a bug, because that is user code that can be
>>>>>> fixed.
>>>>>>
>>>>>> A profile mismatch seems much less like "user code has a bug" and so
>>>>>> doesn't seem to fit any well in the "warning" category.
>>>>>>
>>>>>
>>>>> It is not user code bug, but still be a user bug in how PGO is used.
>>>>>
>>>>
>>>> Is it, though? It sounds like this is expected to some degree (& sounds
>>>> like internally we encourage users to -Wno-error to get around this? Which
>>>> really seems to undermine confidence in Clang's warning quality) - because
>>>> the intent is to apply profiles from previous versions of the program to
>>>> new versions of the program, so things will mismatch, right? And there's
>>>> nothing the user's really meant to do in that case - maybe if it were over
>>>> some threshold.
>>>>
>>>>
>>>>> The warning happens also to be a useful for catch potential compiler
>>>>> bug.
>>>>>
>>>>
>>>>> David
>>>>>
>>>>>
>>>>>>
>>>>>> On the other hand, remarks were specifically designed to be non-bug
>>>>>> important information from optimization passes that users may need to act
>>>>>> on.
>>>>>>
>>>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170720/39e1664d/attachment.html>