[PATCH] D81036: [OpenMP] Begin Adding Analysis Remarks for OpenMP Best Practises.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Cool! Some minor comments below.



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:413
+    return false;
+  }
+
----------------
Early exits are usually easier to read, thus:
```
if (!UV)
 return;
```

---

Maybe we should be more descriptive, telling people that fixed numbers are not future compatible. *And* we should not issue the remark if we find a worksharing loop with a constant trip count (that is a multiple of this constant). I guess the latter part can just be a TODO for now, maybe also a test case:
```
#pragma omp parallel for num_threads(8)
for (int i = 0; i < 8; ++i)
  body(i);
```
That reminds me that we should determine the loop trip count for worksharing loops as well. We could even use that value to feed it into "set_num_threads" to avoid waking more threads than needed.


================
Comment at: llvm/test/Transforms/OpenMP/constant_thread_count_analysis.ll:119
+!57 = !{i64 2, i64 -1, i64 -1, i1 true}
+; ModuleID = 'constant_thread_count_analysis.ll'
----------------
Don't think we need this comment ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81036