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

[David Blaikie via llvm-commits]

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/3e442428/attachment.html>