[Openmp-commits] [PATCH] D64808: [OPENMMP] Resolve lost LoopTripCnt for subsequent loops in same thread.
Alexey Bataev via Phabricator via Openmp-commits
openmp-commits at lists.llvm.org
Tue Jul 16 12:23:23 PDT 2019
ABataev added inline comments.
Comment at: libomptarget/test/offloading/looptripcnt.c:38
+ printf("num_threads %d num_teams %d\n", num_threads, num_teams);
+// 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.
Then better to check for the debug message, I think. Other opinions?
CHANGES SINCE LAST ACTION
More information about the Openmp-commits