[PATCH] D81114: [OpenMPOpt] initial tests for ICV tracking. Only nthreads is used.

[Stefan Stipanovic via Phabricator via llvm-commits]

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


================
Comment at: llvm/test/Transforms/OpenMP/icv_tracking.ll:61
+  tail call void @omp_set_num_threads(i32 10) #1
+  %5 = tail call i32 @omp_get_max_threads() #1
+  tail call void @use(i32 %5) #1
----------------
jdoerfert wrote:
> sstefan1 wrote:
> > jdoerfert wrote:
> > > I guess `%5` should be replaced with `min(%3, 10)`, right? We might want to use SCEV to represent the values and combine them.
> > Not sure I understand why `min(%3, 10)`? Without any transformations `%5` is 10. Maybe I'm missing something?
> > 
> > Will look into using SCEV.
> In reality, it is a value smaller or equal to 10, I think.
> 
> Let's say we have a quad core and the user sets the num_threads to 10, there are two reasonable outcomes. The runtime will cap it at 4, given that any more threads will most likely not help, or it will go with 10. Afaik, OpenMP doesn't guarantee 10 but not more than 10 if you set a limit. We might want to provide user hooks that simplify the logic, e.g., allow us to take 10, but otherwise min(10, %3) seems reasonable to me. WDYT?
Oh, ok. Makes sense now. 10 was totally random. I'll update the comment to have `min(10, %3)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81114