Add 'remark' diagnostic type in LLVM

[Tobias Grosser]

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.

Is this patch OK to commit?

Tobias

-------------- next part --------------
>From 532515ef74297dda5a7c1ae16293d99a75407b48 Mon Sep 17 00:00:00 2001
From: Tobias Grosser <tobias at grosser.es>
Date: Tue, 25 Feb 2014 09:50:23 +0100
Subject: [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.
---
 include/llvm-c/lto.h             | 9 +++++----
 include/llvm/IR/DiagnosticInfo.h | 3 +++
 lib/IR/LLVMContext.cpp           | 3 +++
 lib/LTO/LTOCodeGenerator.cpp     | 3 +++
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/llvm-c/lto.h b/include/llvm-c/lto.h
index bff63ad..92a408f 100644
--- a/include/llvm-c/lto.h
+++ b/include/llvm-c/lto.h
@@ -40,7 +40,7 @@ typedef bool lto_bool_t;
  * @{
  */
 
-#define LTO_API_VERSION 9
+#define LTO_API_VERSION 10
 
 /**
  * \since prior to LTO_API_VERSION=3
@@ -299,9 +299,10 @@ lto_module_get_linkeropt(lto_module_t mod, unsigned int index);
  * \since LTO_API_VERSION=7
  */
 typedef enum {
-  LTO_DS_ERROR,
-  LTO_DS_WARNING,
-  LTO_DS_NOTE
+  LTO_DS_ERROR = 0,
+  LTO_DS_WARNING = 1,
+  LTO_DS_REMARK = 3,
+  LTO_DS_NOTE = 2
 } lto_codegen_diagnostic_severity_t;
 
 /**
diff --git a/include/llvm/IR/DiagnosticInfo.h b/include/llvm/IR/DiagnosticInfo.h
index 4c7af89..9b99ce6 100644
--- a/include/llvm/IR/DiagnosticInfo.h
+++ b/include/llvm/IR/DiagnosticInfo.h
@@ -31,6 +31,9 @@ class Value;
 enum DiagnosticSeverity {
   DS_Error,
   DS_Warning,
+  DS_Remark,
+  // A note attaches additional information to one of the previous diagnostic
+  // types.
   DS_Note
 };
 
diff --git a/lib/IR/LLVMContext.cpp b/lib/IR/LLVMContext.cpp
index d9d6de1..1bfc515 100644
--- a/lib/IR/LLVMContext.cpp
+++ b/lib/IR/LLVMContext.cpp
@@ -142,6 +142,9 @@ void LLVMContext::diagnose(const DiagnosticInfo &DI) {
   case DS_Warning:
     errs() << "warning: " << MsgStorage << "\n";
     break;
+  case DS_Remark:
+    errs() << "remark: " << MsgStorage << "\n";
+    break;
   case DS_Note:
     errs() << "note: " << MsgStorage << "\n";
     break;
diff --git a/lib/LTO/LTOCodeGenerator.cpp b/lib/LTO/LTOCodeGenerator.cpp
index c5f9873..777db9c 100644
--- a/lib/LTO/LTOCodeGenerator.cpp
+++ b/lib/LTO/LTOCodeGenerator.cpp
@@ -562,6 +562,9 @@ void LTOCodeGenerator::DiagnosticHandler2(const DiagnosticInfo &DI) {
   case DS_Warning:
     Severity = LTO_DS_WARNING;
     break;
+  case DS_Remark:
+    Severity = LTO_DS_REMARK;
+    break;
   case DS_Note:
     Severity = LTO_DS_NOTE;
     break;
-- 
1.8.1.2