[PATCH] D69984: [OpenMP][Opt] Annotate known runtime functions and deduplicate more

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert marked an inline comment as done.
jdoerfert added inline comments.


================
Comment at: llvm/test/Transforms/OpenMP/add_attributes.ll:1
+; RUN: opt < %s -S -openmpopt        | FileCheck %s
+; RUN: opt < %s -S -passes=openmpopt | FileCheck %s
----------------
JonChesterfield wrote:
> jdoerfert wrote:
> > JonChesterfield wrote:
> > > Is there a standard in-tree way of deduplicating stuff like this? It could be a hash of function name to expected attribute, with a generator that writes the IR.
> > > 
> > > My last backend generated many thousands of lines of IR from much shorter python scripts. Maybe the in tree equivalent is tablegen?
> > Can you rephrase this comment, I'm not sure I get it.
> > 
> > (btw. this is for attributes not deduplication)
> Sure! Some collision of terminology here.
> 
> This patch is to add attributes to some, but not all functions. So the test case checks that some functions have gained specific function attrs, and that other functions still have no attrs.
> 
> The (de)duplication I refer to is the extra ascii characters in the IR test, not removing duplication function calls. The format of textual IR means the signal to noise ratio is rather worse than, say, 
> 
> ```
> {
>   "void @omp_set_num_threads(i32)" : (nofree, nosync, nounwind, writeonly,),
>   "i32 @omp_get_team_size(i32)" : (nofree, nosync, nounwind, readonly,),
>   "i32 @omp_get_max_task_priority()" : (),
> }```
> 
> or the more concise,
> ```
> # name nofree nosync nounwind readonly writeonly
> {
>   "omp_set_num_threads"     : "x x x _ x",
>   "omp_get_team_size"       : "x x x x _",
>   "omp_get_max_tax_priority : "_ _ _ _ _",
> }
> ```
> 
> The denser table is likely to be easier to scan for errors but can't be directly fed into LLVM, so a dense table + a trivial conversion to IR allows both. Iff there is an established way to do that sort of thing within LLVM. I did lots of this style of generated test for an out of tree target, in very ad hoc python, and wondered if there's an approved equivalent in the open source.
I am unsure why the check lines in the end are bad. They are like your first solution but in 2 instead of one line.
The second solution is actually harder to read (for me).

That being said, I'm open to suggestions on how to improve this ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69984