[Openmp-commits] [PATCH] D64808: [OPENMMP] Resolve lost LoopTripCnt for subsequent loops in same thread.

[Ron Lieberman via Phabricator via Openmp-commits]

ronlieb marked 2 inline comments as done.
ronlieb added inline comments.


================
Comment at: libomptarget/test/offloading/looptripcnt.c:21
+#pragma omp target teams distribute parallel for thread_limit(4)
+  {
+    for (int j = 0; j< N; j++) {
----------------
Hahnfeld wrote:
> I think the standard doesn't expect a structured block, but a `for` loop directly.
will fix shortly.


================
Comment at: libomptarget/test/offloading/looptripcnt.c:38
+  printf("num_threads %d num_teams %d\n", num_threads[0], num_teams[0]);
+// DEBUG: loop trip count is 128
+  return 0;
----------------
ABataev wrote:
> ronlieb wrote:
> > ABataev wrote:
> > > I think, instead of the debug messages better to check the results of the `printf`s.
> > I initially thought about that, but I am worried that depending on what architecture you run on it might produce different numbers of teams, if the thread limit was a lot smaller than on a gpu for example.
> > 
> > but what I am really trying to test is that we found the loopTripCount to pass into the runtime.  
> > 
> > if you strongly prefer that I still do so, I will be happy to change the test. I would of course still keep the DP message for diagnostic purposes down the road.
> It is always better to check the real results rather than debug messages. If you could write a test, which is able to provide a stable check, then do this.
ok, so I spent some time trying to do this. The issue I am encountering is 
that the behavior that needs to be verified is that we properly find the loop trip count in  the function 'target' within omptarget.cpp. This is then passed along to function __tgt_rtl_run_target_team_region in plugins.cpp which  "may" choose to modify the blocksPerGrid or not. So depending on your target, you may see different numbers of teams active if you query omp_get_num_teams().  The observed behavior on different hosts is tenuous at best in determining if loop_trip_count was properly found. The point of this test is to make sure we dont loose the loop_trip_count. In this situation i think it best to catch the problem in Debug build/test where we can explicitly observe the DP message.



Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D64808