Add 'remark' diagnostic type in LLVM

[Tobias Grosser]

On 02/28/2014 06:21 AM, Quentin Colombet wrote:
> Hi Tobias,
>
> On Feb 27, 2014, at 6:23 PM, Tobias Grosser <tobias at grosser.es> wrote:
>
>> On 02/25/2014 08:47 PM, Quentin Colombet wrote:
>>> Hi Tobias,
>>>
>>> On Feb 25, 2014, at 4:19 AM, Tobias Grosser <tobias at grosser.es> wrote:
>>>
>>>> On 02/25/2014 11:48 AM, Alp Toker wrote:
>>>>>
>>>>> On 25/02/2014 10:18, Tobias Grosser wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I would like to add the following patch.
>>>>>>
>>>>>> --------------
>>>>>> Add 'remark' diagnostic type in LLVM
>>>>>>
>>>>>> A 'remark' is information that is not an error or a warning, but
>>>>>> rather some additional information provided to the user. In contrast
>>>>>> to a 'note' a 'remark' is an independent diagnostic, whereas a 'note'
>>>>>> always depends on another diagnostic.
>>>>>>
>>>>>> A typical use case for remark nodes is information provided to the
>>>>>> user, e.g. information provided by the vectorizer about loops that
>>>>>> have been vectorized.
>>>>>
>>>>>
>>>>> One nice thing about warnings is that they can be suppressed with -Wno
>>>>> or upgraded to errors with -Werror= and effort has been made to ensure
>>>>> each warning has an associated option (mostly useful for built-in
>>>>> diagnostics).
>>>>>
>>>>> I'm guessing this remark capability is intended to support Quentin's
>>>>> backend diagnostics -- could you provide some background on the planned
>>>>> mechanism to enable or suppress them?
>>>>
>>>> Hi Alp,
>>>>
>>>> yes, this is intended for Quentin's backend diagnostics and the question you ask is very valid.
>>>>
>>>> The current diagnostics emitted in the backend are all warnings or errors and are controlled by explicit flags (-Wbackend-plugin, -Winline-asm,..). This is already taken care of.
>>>>
>>>> For the 'remark' diagnostics I just proposed I did not yet add any flags. I also do not think it makes sense to use -Werror or something to promote those remarks to errors.
>>> Agreed.
>>>
>>>>
>>>> However, enabling them using command line options is necessary. I see two approaches here:
>>>>
>>>> 1) Model them similar to the -W flags
>>>>
>>>> We could use something like '-Rvector '-Rno-vector' to enable these diagnostics. And then allow more fine-grained control with '-Rloop-vector' or '-Rslp-vector'.
>>>>
>>>> -R does not yet have any meaning for clang and gcc, so using it for this purpose may be fine. This may also give us another data-point for the 'info' vs. 'remark' discussion currently going on as "-I" is already taken.
>>>>
>>>> To later enable different verbosity modes, we could use a formulation such as '-Rvector=3' with '-Rno-vector' equal to '-Rvector=0’.
>>> I have a preference for this approach. This will be easier to present the information in a consistent way to the user.
>>> On the other hand, we may want to be compatible with gcc flags. But I guess we can produce some aliases if this is a requirement.
>>>
>>> Regarding the patch itself.
>>> Assuming we pursue in that direction, this is breaking the API compatibility:
>>>     LTO_DS_WARNING,
>>> +  LTO_DS_REMARK,
>>>     LTO_DS_NOTE
>>>   } lto_codegen_diagnostic_severity_t;
>>>
>>> More sure to bump the LTO API version with that change.
>>
>> Hi Quentin,
>>
>> as we now agreed to introduce 'remark' diagnostics, here an updated patch. The only change is the bump of the LTO API as well as the use of explicit values for this enum.
> @@ -299,9 +299,10 @@ lto_module_get_linkeropt(lto_module_t mod, unsigned int index);
>    * \since LTO_API_VERSION=7
>    */
> I’ll add a comment saying REMARK was adding since LTO_API_VERSION=xxx

Done.

> Other than that: LGTM.

Committed in r202474.

Tobias