[PATCH] D72425: [OptRemark] RFC: Introduce a message table for OptRemarks

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added a comment.

This is great! Sorry for the delay. More comments inline.



================
Comment at: llvm/include/llvm/Remarks/OptRemarkDiagBase.td:23
+
+def Kind_General : OptRemarkKind<"General">;
+def Kind_Missed : OptRemarkKind<"Missed">;
----------------
I think a few places call this "Passed". Would that be better than "General"?


================
Comment at: llvm/include/llvm/Remarks/OptRemarkDiagBase.td:33
+  string VerboseFormatStr = FormatVerbose;
+  OptRemarkGroup Group = ?;
+  OptRemarkKind  Kind = Kind_General;
----------------
In this scheme, it will be hard for a remark to be part of multiple groups, right?

I was imagining something where we can do things like:
```
def gvn_load_elim : OptRemark<"LoadElim", "...">;
def gvn_load_elim_missed : OptRemark<"LoadElimMissed", "...">, Kind<Kind_Missed>;
def licm_hoisted : OptRemark<"InstHoisted", "...">;
def loop_vectorized : OptRemark<"LoopVectorized", "...">;

def GroupGVN : OptRemarkGroup<"GVN", [ gvn_load_elim, gvn_load_elim_missed ]>;
def GroupLICM : OptRemarkGroup<"LICM", [ licm_hoisted ]>;
def GroupLV : OptRemarkGroup<"LV", [ loop_vectorized ]>;
def GroupLoopStuff : OptRemarkGroup<"LoopStuff", [ licm_hoisted, loop_vectorized ]>;
```

or even groups of groups to avoid listing everything:

```
def GroupLoopStuff : OptRemarkGroup<"LoopStuff", [ GroupLICM, GroupLoopStuff ]>;
```

where we would have the liberty of putting remarks in different groups:

* for compiler devs: group remarks by pass
* for users: group remarks by a higher level concept: loop optimizations, actionability, sanitizer-added code, etc.

I don't know how hard it is to get something like this, but let me know what you think.


================
Comment at: llvm/test/TableGen/opt-remark-diag.td:20
+
+// CHECK-ENUMS: enum OptRemarkDiagGroup_Test1 {
+// CHECK-ENUMS:   RemarkName = 10000,
----------------
What would be the use case of these enums? Can't the same be achieved by not quoting `RemarkName` and `Test1` in the `OPT_REMARK(` macros like in `clang/include/clang/Driver/Options.h`:

```
enum ID {
    OPT_INVALID = 0, // This is not an option ID.
#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
               HELPTEXT, METAVAR, VALUES)                                      \
  OPT_##ID,
#include "clang/Driver/Options.inc"
    LastOption
#undef OPTION
  };
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72425/new/

https://reviews.llvm.org/D72425