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

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 20 10:28:15 PDT 2017


I am fine making it remarks, but I think there needs to an option to force
it to be warning or even error.

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/30cf442c/attachment.html>


More information about the llvm-commits mailing list